You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by ja...@apache.org on 2012/11/02 14:02:55 UTC
[2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Only return X-Couch-Id (rev is available in ETag)
Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/4edbb93d
Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/4edbb93d
Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/4edbb93d
Branch: refs/heads/master
Commit: 4edbb93d2271ac1eb82f4d2bb072b8bdf6829f85
Parents: 0a64f31
Author: Benjamin Nortier <bj...@gmail.com>
Authored: Fri Sep 21 16:46:46 2012 +0100
Committer: Jan Lehnardt <ja...@apache.org>
Committed: Fri Nov 2 14:02:48 2012 +0100
----------------------------------------------------------------------
src/couchdb/couch_httpd.erl | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/couchdb/blob/4edbb93d/src/couchdb/couch_httpd.erl
----------------------------------------------------------------------
diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl
index da47dfc..eb35ff9 100644
--- a/src/couchdb/couch_httpd.erl
+++ b/src/couchdb/couch_httpd.erl
@@ -692,19 +692,19 @@ send_json(Req, Code, Headers, Value) ->
{"Content-Type", negotiate_content_type(Req)},
{"Cache-Control", "must-revalidate"}
],
- IdAndRevHeaders = case Value of
- {Props} when is_list(Props) ->
- case {lists:keyfind(id, 1, Props), lists:keyfind(rev, 1, Props)} of
- {{_, Id}, {_, Rev}} ->
- [{"X-Couch-Id", Id}, {"X-Couch-Rev", Rev}];
- _ ->
- []
- end;
- _ ->
- []
- end,
+ IdHeader = case Value of
+ {Props} when is_list(Props) ->
+ case lists:keyfind(id, 1, Props) of
+ {_, Id} ->
+ [{"X-Couch-Id", Id}];
+ _ ->
+ []
+ end;
+ _ ->
+ []
+ end,
Body = [start_jsonp(), ?JSON_ENCODE(Value), end_jsonp(), $\n],
- send_response(Req, Code, DefaultHeaders ++ IdAndRevHeaders ++ Headers, Body).
+ send_response(Req, Code, DefaultHeaders ++ IdHeader ++ Headers, Body).
start_json_response(Req, Code) ->
start_json_response(Req, Code, []).
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Randall Leeds <ra...@gmail.com>.
On Fri, Nov 2, 2012 at 6:14 PM, Adam Kocoloski <ko...@apache.org> wrote:
> On Nov 2, 2012, at 2:47 PM, Randall Leeds <ra...@gmail.com> wrote:
>
> > On Fri, Nov 2, 2012 at 6:50 AM, Robert Newson <rn...@apache.org>
> wrote:
> >
> >> Sure, and it couldn't hurt to clarify how we should use git in these
> >> circumstances. The principal rule is that each commit should be
> >> self-contained and, further, that it should at least potentially be
> worth
> >> reading in the future. A bug in a branch, found during a review and then
> >> fixed, does not to be preserved and should be squashed into a single
> >> commit. I would expect most bugfix branches to have a single commit, and
> >> most feature branches to have several (where each is incrementally
> >> introducing the feature, or refactoring to make way for it). And, of
> >> course, we need the prohibition on merge commits lifted so that can
> >> preserve that history and record the point at which it joined master.
> >>
> >
> > Unfortunately, once the Pull Request is opened, it's hard to modify it.
> > With review, it's easy to add more commits, but I do not think asking
> > contributors to force push their PR branch is a good idea.
> >
> > From the Pull Request UI on GitHub, the Files Changed tab is essentially
> > the squashed view, while the Commits tab shows the individual commits. I
> > tend to review the squashed view.
> >
> > From here, one possible best flow is for the *committer* to squash the
> > branch in the merge and add the appropriate "close #" or "fix #" to the
> > commit message, so that the PR is automatically closed. Of course, that
> > loses the authorship information. So one could do a "git merge --squash"
> > followed by a "git commit --author=".
> >
> > But I wonder if squashing just isn't the wrong thing to do anyway. If we
> > allow merge commits, instead, it becomes possible to use "git log
> --merges"
> > to get the nice, squashy history, but we don't have to muck around with
> > anything at all. I've even heard people make the case for *always* using
> > --no-ff on every merge.
> >
> > "git log --first-parent" might be even more useful, since (if I'm not
> > mistaken) it would show fast-forward pushes committers have made straight
> > to the repo, but only show the merge commits from pull requests and
> feature
> > merges.
> >
> > I think if we want to play nice with pull requests, we may want to
> > re-evaluate our no-merges policy.
>
> That all makes a ton of sense to me, but I'm sure I've forgotten the very
> good reasons that we had for not using merge commits in the beginning.
>
> Adam
The only one I can recall is that we needed a way to go back to SVN if
infra deemed the git experiment a failure. That meant linear history.
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Adam Kocoloski <ko...@apache.org>.
On Nov 2, 2012, at 2:47 PM, Randall Leeds <ra...@gmail.com> wrote:
> On Fri, Nov 2, 2012 at 6:50 AM, Robert Newson <rn...@apache.org> wrote:
>
>> Sure, and it couldn't hurt to clarify how we should use git in these
>> circumstances. The principal rule is that each commit should be
>> self-contained and, further, that it should at least potentially be worth
>> reading in the future. A bug in a branch, found during a review and then
>> fixed, does not to be preserved and should be squashed into a single
>> commit. I would expect most bugfix branches to have a single commit, and
>> most feature branches to have several (where each is incrementally
>> introducing the feature, or refactoring to make way for it). And, of
>> course, we need the prohibition on merge commits lifted so that can
>> preserve that history and record the point at which it joined master.
>>
>
> Unfortunately, once the Pull Request is opened, it's hard to modify it.
> With review, it's easy to add more commits, but I do not think asking
> contributors to force push their PR branch is a good idea.
>
> From the Pull Request UI on GitHub, the Files Changed tab is essentially
> the squashed view, while the Commits tab shows the individual commits. I
> tend to review the squashed view.
>
> From here, one possible best flow is for the *committer* to squash the
> branch in the merge and add the appropriate "close #" or "fix #" to the
> commit message, so that the PR is automatically closed. Of course, that
> loses the authorship information. So one could do a "git merge --squash"
> followed by a "git commit --author=".
>
> But I wonder if squashing just isn't the wrong thing to do anyway. If we
> allow merge commits, instead, it becomes possible to use "git log --merges"
> to get the nice, squashy history, but we don't have to muck around with
> anything at all. I've even heard people make the case for *always* using
> --no-ff on every merge.
>
> "git log --first-parent" might be even more useful, since (if I'm not
> mistaken) it would show fast-forward pushes committers have made straight
> to the repo, but only show the merge commits from pull requests and feature
> merges.
>
> I think if we want to play nice with pull requests, we may want to
> re-evaluate our no-merges policy.
That all makes a ton of sense to me, but I'm sure I've forgotten the very good reasons that we had for not using merge commits in the beginning.
Adam
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Randall Leeds <ra...@gmail.com>.
On Fri, Nov 2, 2012 at 6:50 AM, Robert Newson <rn...@apache.org> wrote:
> Sure, and it couldn't hurt to clarify how we should use git in these
> circumstances. The principal rule is that each commit should be
> self-contained and, further, that it should at least potentially be worth
> reading in the future. A bug in a branch, found during a review and then
> fixed, does not to be preserved and should be squashed into a single
> commit. I would expect most bugfix branches to have a single commit, and
> most feature branches to have several (where each is incrementally
> introducing the feature, or refactoring to make way for it). And, of
> course, we need the prohibition on merge commits lifted so that can
> preserve that history and record the point at which it joined master.
>
Unfortunately, once the Pull Request is opened, it's hard to modify it.
With review, it's easy to add more commits, but I do not think asking
contributors to force push their PR branch is a good idea.
>From the Pull Request UI on GitHub, the Files Changed tab is essentially
the squashed view, while the Commits tab shows the individual commits. I
tend to review the squashed view.
>From here, one possible best flow is for the *committer* to squash the
branch in the merge and add the appropriate "close #" or "fix #" to the
commit message, so that the PR is automatically closed. Of course, that
loses the authorship information. So one could do a "git merge --squash"
followed by a "git commit --author=".
But I wonder if squashing just isn't the wrong thing to do anyway. If we
allow merge commits, instead, it becomes possible to use "git log --merges"
to get the nice, squashy history, but we don't have to muck around with
anything at all. I've even heard people make the case for *always* using
--no-ff on every merge.
"git log --first-parent" might be even more useful, since (if I'm not
mistaken) it would show fast-forward pushes committers have made straight
to the repo, but only show the merge commits from pull requests and feature
merges.
I think if we want to play nice with pull requests, we may want to
re-evaluate our no-merges policy.
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Robert Newson <rn...@apache.org>.
Sure, and it couldn't hurt to clarify how we should use git in these
circumstances. The principal rule is that each commit should be
self-contained and, further, that it should at least potentially be worth
reading in the future. A bug in a branch, found during a review and then
fixed, does not to be preserved and should be squashed into a single
commit. I would expect most bugfix branches to have a single commit, and
most feature branches to have several (where each is incrementally
introducing the feature, or refactoring to make way for it). And, of
course, we need the prohibition on merge commits lifted so that can
preserve that history and record the point at which it joined master.
On 2 November 2012 13:42, Jan Lehnardt <ja...@apache.org> wrote:
>
> On Nov 2, 2012, at 14:41 , Robert Newson <rn...@apache.org> wrote:
>
> > Yup, it's introduced and removed during the PR, a squash would have been
> > useful here, it makes for silly history. Or a merge, but that requires
> > unblocking a commit hook (though we require that after 1.3 anyway)
>
> we need better instructions for this in the email that gets sent to dev@
> when the PR is created. I'll try to work on this with Paul next week at
> ApacheCon EU.
>
>
> >
> >
> > On 2 November 2012 13:40, Jan Lehnardt <ja...@apache.org> wrote:
> >
> >>
> >> On Nov 2, 2012, at 14:37 , Jan Lehnardt <ja...@apache.org> wrote:
> >>
> >>>
> >>> On Nov 2, 2012, at 14:02 , jan@apache.org wrote:
> >>>
> >>>> Only return X-Couch-Id (rev is available in ETag)
> >>>>
> >>>>
> >>>> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
> >>>> Commit:
> http://git-wip-us.apache.org/repos/asf/couchdb/commit/4edbb93d
> >>>> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/4edbb93d
> >>>> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/4edbb93d
> >>>>
> >>>> Branch: refs/heads/master
> >>>> Commit: 4edbb93d2271ac1eb82f4d2bb072b8bdf6829f85
> >>>> Parents: 0a64f31
> >>>> Author: Benjamin Nortier <bj...@gmail.com>
> >>>> Authored: Fri Sep 21 16:46:46 2012 +0100
> >>>> Committer: Jan Lehnardt <ja...@apache.org>
> >>>> Committed: Fri Nov 2 14:02:48 2012 +0100
> >>>>
> >>>> ----------------------------------------------------------------------
> >>>> src/couchdb/couch_httpd.erl | 24 ++++++++++++------------
> >>>> 1 files changed, 12 insertions(+), 12 deletions(-)
> >>>> ----------------------------------------------------------------------
> >>>>
> >>>>
> >>>>
> >>
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/4edbb93d/src/couchdb/couch_httpd.erl
> >>>> ----------------------------------------------------------------------
> >>>> diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl
> >>>> index da47dfc..eb35ff9 100644
> >>>> --- a/src/couchdb/couch_httpd.erl
> >>>> +++ b/src/couchdb/couch_httpd.erl
> >>>> @@ -692,19 +692,19 @@ send_json(Req, Code, Headers, Value) ->
> >>>> {"Content-Type", negotiate_content_type(Req)},
> >>>> {"Cache-Control", "must-revalidate"}
> >>>> ],
> >>>> - IdAndRevHeaders = case Value of
> >>>> - {Props} when is_list(Props) ->
> >>>> - case {lists:keyfind(id, 1, Props),
> >> lists:keyfind(rev, 1, Props)} of
> >>>> - {{_, Id}, {_, Rev}} ->
> >>>> - [{"X-Couch-Id", Id}, {"X-Couch-Rev",
> >> Rev}];
> >>>> - _ ->
> >>>> - []
> >>>> - end;
> >>>> - _ ->
> >>>> - []
> >>>> - end,
> >>>
> >>> Curious issue: GitHub doesn’t show this “-” section on
> >> https://github.com/apache/couchdb/pull/32/files
> >>>
> >>> Only on
> >>
> https://github.com/bjnortier/couchdb/commit/b38374034a57db924a2650038f078dbe4c61b715
> >>>
> >>> I based my review on the former. Sorry for accidentally committing this
> >> wrongly.
> >>>
> >>> I have to pop out for a few hours, if anyone feels like reverting this,
> >> I’d be much obliged.
> >>>
> >>> Thanks to @rnewson for spotting this!
> >>
> >>
> >> nevermind, jumped the gun, this was re-added in
> >>
> https://github.com/bjnortier/couchdb/commit/fe934a16760dcbf975d9f8b4923eee53747bfd25
> >>
> >> Cheers
> >> Jan
> >> --
> >>
> >>> Jan
> >>> --
> >>>
> >>>
> >>>> + IdHeader = case Value of
> >>>> + {Props} when is_list(Props) ->
> >>>> + case lists:keyfind(id, 1, Props) of
> >>>> + {_, Id} ->
> >>>> + [{"X-Couch-Id", Id}];
> >>>> + _ ->
> >>>> + []
> >>>> + end;
> >>>> + _ ->
> >>>> + []
> >>>> + end,
> >>>> Body = [start_jsonp(), ?JSON_ENCODE(Value), end_jsonp(), $\n],
> >>>> - send_response(Req, Code, DefaultHeaders ++ IdAndRevHeaders ++
> >> Headers, Body).
> >>>> + send_response(Req, Code, DefaultHeaders ++ IdHeader ++ Headers,
> >> Body).
> >>>>
> >>>> start_json_response(Req, Code) ->
> >>>> start_json_response(Req, Code, []).
> >>>>
> >>>
> >>
> >>
>
>
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by "matt j. sorenson" <ma...@sorensonbros.net>.
On Fri, Nov 2, 2012 at 8:42 AM, Jan Lehnardt <ja...@apache.org> wrote:
>
> On Nov 2, 2012, at 14:41 , Robert Newson <rn...@apache.org> wrote:
>
> > Yup, it's introduced and removed during the PR, a squash would have been
> > useful here, it makes for silly history. Or a merge, but that requires
> > unblocking a commit hook (though we require that after 1.3 anyway)
>
> we need better instructions for this in the email that gets sent to dev@
> when the PR is created. I'll try to work on this with Paul next week at
> ApacheCon EU.
I hadn't previously been oriented with 'squashing' in git until rnewson
brought it up (so thanks!) and just now I caught steve klabnik tweeting
about it...
http://blog.steveklabnik.com/posts/2012-11-08-how-to-squash-commits-in-a-github-pull-request
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Jan Lehnardt <ja...@apache.org>.
On Nov 2, 2012, at 14:41 , Robert Newson <rn...@apache.org> wrote:
> Yup, it's introduced and removed during the PR, a squash would have been
> useful here, it makes for silly history. Or a merge, but that requires
> unblocking a commit hook (though we require that after 1.3 anyway)
we need better instructions for this in the email that gets sent to dev@
when the PR is created. I'll try to work on this with Paul next week at
ApacheCon EU.
>
>
> On 2 November 2012 13:40, Jan Lehnardt <ja...@apache.org> wrote:
>
>>
>> On Nov 2, 2012, at 14:37 , Jan Lehnardt <ja...@apache.org> wrote:
>>
>>>
>>> On Nov 2, 2012, at 14:02 , jan@apache.org wrote:
>>>
>>>> Only return X-Couch-Id (rev is available in ETag)
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
>>>> Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/4edbb93d
>>>> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/4edbb93d
>>>> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/4edbb93d
>>>>
>>>> Branch: refs/heads/master
>>>> Commit: 4edbb93d2271ac1eb82f4d2bb072b8bdf6829f85
>>>> Parents: 0a64f31
>>>> Author: Benjamin Nortier <bj...@gmail.com>
>>>> Authored: Fri Sep 21 16:46:46 2012 +0100
>>>> Committer: Jan Lehnardt <ja...@apache.org>
>>>> Committed: Fri Nov 2 14:02:48 2012 +0100
>>>>
>>>> ----------------------------------------------------------------------
>>>> src/couchdb/couch_httpd.erl | 24 ++++++++++++------------
>>>> 1 files changed, 12 insertions(+), 12 deletions(-)
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>>
>> http://git-wip-us.apache.org/repos/asf/couchdb/blob/4edbb93d/src/couchdb/couch_httpd.erl
>>>> ----------------------------------------------------------------------
>>>> diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl
>>>> index da47dfc..eb35ff9 100644
>>>> --- a/src/couchdb/couch_httpd.erl
>>>> +++ b/src/couchdb/couch_httpd.erl
>>>> @@ -692,19 +692,19 @@ send_json(Req, Code, Headers, Value) ->
>>>> {"Content-Type", negotiate_content_type(Req)},
>>>> {"Cache-Control", "must-revalidate"}
>>>> ],
>>>> - IdAndRevHeaders = case Value of
>>>> - {Props} when is_list(Props) ->
>>>> - case {lists:keyfind(id, 1, Props),
>> lists:keyfind(rev, 1, Props)} of
>>>> - {{_, Id}, {_, Rev}} ->
>>>> - [{"X-Couch-Id", Id}, {"X-Couch-Rev",
>> Rev}];
>>>> - _ ->
>>>> - []
>>>> - end;
>>>> - _ ->
>>>> - []
>>>> - end,
>>>
>>> Curious issue: GitHub doesn’t show this “-” section on
>> https://github.com/apache/couchdb/pull/32/files
>>>
>>> Only on
>> https://github.com/bjnortier/couchdb/commit/b38374034a57db924a2650038f078dbe4c61b715
>>>
>>> I based my review on the former. Sorry for accidentally committing this
>> wrongly.
>>>
>>> I have to pop out for a few hours, if anyone feels like reverting this,
>> I’d be much obliged.
>>>
>>> Thanks to @rnewson for spotting this!
>>
>>
>> nevermind, jumped the gun, this was re-added in
>> https://github.com/bjnortier/couchdb/commit/fe934a16760dcbf975d9f8b4923eee53747bfd25
>>
>> Cheers
>> Jan
>> --
>>
>>> Jan
>>> --
>>>
>>>
>>>> + IdHeader = case Value of
>>>> + {Props} when is_list(Props) ->
>>>> + case lists:keyfind(id, 1, Props) of
>>>> + {_, Id} ->
>>>> + [{"X-Couch-Id", Id}];
>>>> + _ ->
>>>> + []
>>>> + end;
>>>> + _ ->
>>>> + []
>>>> + end,
>>>> Body = [start_jsonp(), ?JSON_ENCODE(Value), end_jsonp(), $\n],
>>>> - send_response(Req, Code, DefaultHeaders ++ IdAndRevHeaders ++
>> Headers, Body).
>>>> + send_response(Req, Code, DefaultHeaders ++ IdHeader ++ Headers,
>> Body).
>>>>
>>>> start_json_response(Req, Code) ->
>>>> start_json_response(Req, Code, []).
>>>>
>>>
>>
>>
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Robert Newson <rn...@apache.org>.
Yup, it's introduced and removed during the PR, a squash would have been
useful here, it makes for silly history. Or a merge, but that requires
unblocking a commit hook (though we require that after 1.3 anyway)
On 2 November 2012 13:40, Jan Lehnardt <ja...@apache.org> wrote:
>
> On Nov 2, 2012, at 14:37 , Jan Lehnardt <ja...@apache.org> wrote:
>
> >
> > On Nov 2, 2012, at 14:02 , jan@apache.org wrote:
> >
> >> Only return X-Couch-Id (rev is available in ETag)
> >>
> >>
> >> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
> >> Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/4edbb93d
> >> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/4edbb93d
> >> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/4edbb93d
> >>
> >> Branch: refs/heads/master
> >> Commit: 4edbb93d2271ac1eb82f4d2bb072b8bdf6829f85
> >> Parents: 0a64f31
> >> Author: Benjamin Nortier <bj...@gmail.com>
> >> Authored: Fri Sep 21 16:46:46 2012 +0100
> >> Committer: Jan Lehnardt <ja...@apache.org>
> >> Committed: Fri Nov 2 14:02:48 2012 +0100
> >>
> >> ----------------------------------------------------------------------
> >> src/couchdb/couch_httpd.erl | 24 ++++++++++++------------
> >> 1 files changed, 12 insertions(+), 12 deletions(-)
> >> ----------------------------------------------------------------------
> >>
> >>
> >>
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/4edbb93d/src/couchdb/couch_httpd.erl
> >> ----------------------------------------------------------------------
> >> diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl
> >> index da47dfc..eb35ff9 100644
> >> --- a/src/couchdb/couch_httpd.erl
> >> +++ b/src/couchdb/couch_httpd.erl
> >> @@ -692,19 +692,19 @@ send_json(Req, Code, Headers, Value) ->
> >> {"Content-Type", negotiate_content_type(Req)},
> >> {"Cache-Control", "must-revalidate"}
> >> ],
> >> - IdAndRevHeaders = case Value of
> >> - {Props} when is_list(Props) ->
> >> - case {lists:keyfind(id, 1, Props),
> lists:keyfind(rev, 1, Props)} of
> >> - {{_, Id}, {_, Rev}} ->
> >> - [{"X-Couch-Id", Id}, {"X-Couch-Rev",
> Rev}];
> >> - _ ->
> >> - []
> >> - end;
> >> - _ ->
> >> - []
> >> - end,
> >
> > Curious issue: GitHub doesn’t show this “-” section on
> https://github.com/apache/couchdb/pull/32/files
> >
> > Only on
> https://github.com/bjnortier/couchdb/commit/b38374034a57db924a2650038f078dbe4c61b715
> >
> > I based my review on the former. Sorry for accidentally committing this
> wrongly.
> >
> > I have to pop out for a few hours, if anyone feels like reverting this,
> I’d be much obliged.
> >
> > Thanks to @rnewson for spotting this!
>
>
> nevermind, jumped the gun, this was re-added in
> https://github.com/bjnortier/couchdb/commit/fe934a16760dcbf975d9f8b4923eee53747bfd25
>
> Cheers
> Jan
> --
>
> > Jan
> > --
> >
> >
> >> + IdHeader = case Value of
> >> + {Props} when is_list(Props) ->
> >> + case lists:keyfind(id, 1, Props) of
> >> + {_, Id} ->
> >> + [{"X-Couch-Id", Id}];
> >> + _ ->
> >> + []
> >> + end;
> >> + _ ->
> >> + []
> >> + end,
> >> Body = [start_jsonp(), ?JSON_ENCODE(Value), end_jsonp(), $\n],
> >> - send_response(Req, Code, DefaultHeaders ++ IdAndRevHeaders ++
> Headers, Body).
> >> + send_response(Req, Code, DefaultHeaders ++ IdHeader ++ Headers,
> Body).
> >>
> >> start_json_response(Req, Code) ->
> >> start_json_response(Req, Code, []).
> >>
> >
>
>
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Jan Lehnardt <ja...@apache.org>.
On Nov 2, 2012, at 14:37 , Jan Lehnardt <ja...@apache.org> wrote:
>
> On Nov 2, 2012, at 14:02 , jan@apache.org wrote:
>
>> Only return X-Couch-Id (rev is available in ETag)
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/4edbb93d
>> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/4edbb93d
>> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/4edbb93d
>>
>> Branch: refs/heads/master
>> Commit: 4edbb93d2271ac1eb82f4d2bb072b8bdf6829f85
>> Parents: 0a64f31
>> Author: Benjamin Nortier <bj...@gmail.com>
>> Authored: Fri Sep 21 16:46:46 2012 +0100
>> Committer: Jan Lehnardt <ja...@apache.org>
>> Committed: Fri Nov 2 14:02:48 2012 +0100
>>
>> ----------------------------------------------------------------------
>> src/couchdb/couch_httpd.erl | 24 ++++++++++++------------
>> 1 files changed, 12 insertions(+), 12 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/couchdb/blob/4edbb93d/src/couchdb/couch_httpd.erl
>> ----------------------------------------------------------------------
>> diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl
>> index da47dfc..eb35ff9 100644
>> --- a/src/couchdb/couch_httpd.erl
>> +++ b/src/couchdb/couch_httpd.erl
>> @@ -692,19 +692,19 @@ send_json(Req, Code, Headers, Value) ->
>> {"Content-Type", negotiate_content_type(Req)},
>> {"Cache-Control", "must-revalidate"}
>> ],
>> - IdAndRevHeaders = case Value of
>> - {Props} when is_list(Props) ->
>> - case {lists:keyfind(id, 1, Props), lists:keyfind(rev, 1, Props)} of
>> - {{_, Id}, {_, Rev}} ->
>> - [{"X-Couch-Id", Id}, {"X-Couch-Rev", Rev}];
>> - _ ->
>> - []
>> - end;
>> - _ ->
>> - []
>> - end,
>
> Curious issue: GitHub doesn’t show this “-” section on https://github.com/apache/couchdb/pull/32/files
>
> Only on https://github.com/bjnortier/couchdb/commit/b38374034a57db924a2650038f078dbe4c61b715
>
> I based my review on the former. Sorry for accidentally committing this wrongly.
>
> I have to pop out for a few hours, if anyone feels like reverting this, I’d be much obliged.
>
> Thanks to @rnewson for spotting this!
nevermind, jumped the gun, this was re-added in https://github.com/bjnortier/couchdb/commit/fe934a16760dcbf975d9f8b4923eee53747bfd25
Cheers
Jan
--
> Jan
> --
>
>
>> + IdHeader = case Value of
>> + {Props} when is_list(Props) ->
>> + case lists:keyfind(id, 1, Props) of
>> + {_, Id} ->
>> + [{"X-Couch-Id", Id}];
>> + _ ->
>> + []
>> + end;
>> + _ ->
>> + []
>> + end,
>> Body = [start_jsonp(), ?JSON_ENCODE(Value), end_jsonp(), $\n],
>> - send_response(Req, Code, DefaultHeaders ++ IdAndRevHeaders ++ Headers, Body).
>> + send_response(Req, Code, DefaultHeaders ++ IdHeader ++ Headers, Body).
>>
>> start_json_response(Req, Code) ->
>> start_json_response(Req, Code, []).
>>
>
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Jan Lehnardt <ja...@apache.org>.
On Nov 2, 2012, at 14:02 , jan@apache.org wrote:
> Only return X-Couch-Id (rev is available in ETag)
>
>
> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
> Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/4edbb93d
> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/4edbb93d
> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/4edbb93d
>
> Branch: refs/heads/master
> Commit: 4edbb93d2271ac1eb82f4d2bb072b8bdf6829f85
> Parents: 0a64f31
> Author: Benjamin Nortier <bj...@gmail.com>
> Authored: Fri Sep 21 16:46:46 2012 +0100
> Committer: Jan Lehnardt <ja...@apache.org>
> Committed: Fri Nov 2 14:02:48 2012 +0100
>
> ----------------------------------------------------------------------
> src/couchdb/couch_httpd.erl | 24 ++++++++++++------------
> 1 files changed, 12 insertions(+), 12 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/4edbb93d/src/couchdb/couch_httpd.erl
> ----------------------------------------------------------------------
> diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl
> index da47dfc..eb35ff9 100644
> --- a/src/couchdb/couch_httpd.erl
> +++ b/src/couchdb/couch_httpd.erl
> @@ -692,19 +692,19 @@ send_json(Req, Code, Headers, Value) ->
> {"Content-Type", negotiate_content_type(Req)},
> {"Cache-Control", "must-revalidate"}
> ],
> - IdAndRevHeaders = case Value of
> - {Props} when is_list(Props) ->
> - case {lists:keyfind(id, 1, Props), lists:keyfind(rev, 1, Props)} of
> - {{_, Id}, {_, Rev}} ->
> - [{"X-Couch-Id", Id}, {"X-Couch-Rev", Rev}];
> - _ ->
> - []
> - end;
> - _ ->
> - []
> - end,
Curious issue: GitHub doesn’t show this “-” section on https://github.com/apache/couchdb/pull/32/files
Only on https://github.com/bjnortier/couchdb/commit/b38374034a57db924a2650038f078dbe4c61b715
I based my review on the former. Sorry for accidentally committing this wrongly.
I have to pop out for a few hours, if anyone feels like reverting this, I’d be much obliged.
Thanks to @rnewson for spotting this!
Jan
--
> + IdHeader = case Value of
> + {Props} when is_list(Props) ->
> + case lists:keyfind(id, 1, Props) of
> + {_, Id} ->
> + [{"X-Couch-Id", Id}];
> + _ ->
> + []
> + end;
> + _ ->
> + []
> + end,
> Body = [start_jsonp(), ?JSON_ENCODE(Value), end_jsonp(), $\n],
> - send_response(Req, Code, DefaultHeaders ++ IdAndRevHeaders ++ Headers, Body).
> + send_response(Req, Code, DefaultHeaders ++ IdHeader ++ Headers, Body).
>
> start_json_response(Req, Code) ->
> start_json_response(Req, Code, []).
>
Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Posted by Robert Newson <rn...@apache.org>.
Removing X-Couch-Rev breaks backwards compatibility, obliging the next
release to be 2.0. Intentional? Also, what ticket is this?
On 2 November 2012 13:02, <ja...@apache.org> wrote:
> Only return X-Couch-Id (rev is available in ETag)
>
>
> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
> Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/4edbb93d
> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/4edbb93d
> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/4edbb93d
>
> Branch: refs/heads/master
> Commit: 4edbb93d2271ac1eb82f4d2bb072b8bdf6829f85
> Parents: 0a64f31
> Author: Benjamin Nortier <bj...@gmail.com>
> Authored: Fri Sep 21 16:46:46 2012 +0100
> Committer: Jan Lehnardt <ja...@apache.org>
> Committed: Fri Nov 2 14:02:48 2012 +0100
>
> ----------------------------------------------------------------------
> src/couchdb/couch_httpd.erl | 24 ++++++++++++------------
> 1 files changed, 12 insertions(+), 12 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/4edbb93d/src/couchdb/couch_httpd.erl
> ----------------------------------------------------------------------
> diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl
> index da47dfc..eb35ff9 100644
> --- a/src/couchdb/couch_httpd.erl
> +++ b/src/couchdb/couch_httpd.erl
> @@ -692,19 +692,19 @@ send_json(Req, Code, Headers, Value) ->
> {"Content-Type", negotiate_content_type(Req)},
> {"Cache-Control", "must-revalidate"}
> ],
> - IdAndRevHeaders = case Value of
> - {Props} when is_list(Props) ->
> - case {lists:keyfind(id, 1, Props),
> lists:keyfind(rev, 1, Props)} of
> - {{_, Id}, {_, Rev}} ->
> - [{"X-Couch-Id", Id}, {"X-Couch-Rev", Rev}];
> - _ ->
> - []
> - end;
> - _ ->
> - []
> - end,
> + IdHeader = case Value of
> + {Props} when is_list(Props) ->
> + case lists:keyfind(id, 1, Props) of
> + {_, Id} ->
> + [{"X-Couch-Id", Id}];
> + _ ->
> + []
> + end;
> + _ ->
> + []
> + end,
> Body = [start_jsonp(), ?JSON_ENCODE(Value), end_jsonp(), $\n],
> - send_response(Req, Code, DefaultHeaders ++ IdAndRevHeaders ++
> Headers, Body).
> + send_response(Req, Code, DefaultHeaders ++ IdHeader ++ Headers, Body).
>
> start_json_response(Req, Code) ->
> start_json_response(Req, Code, []).
>
>