incubator-couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Newson <rnew...@apache.org>
Subject Re: [2/3] git commit: Only return X-Couch-Id (rev is available in ETag)
Date Fri, 02 Nov 2012 13:50:32 GMT
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 <jan@apache.org> wrote:

>
> On Nov 2, 2012, at 14:41 , Robert Newson <rnewson@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 <jan@apache.org> wrote:
> >
> >>
> >> On Nov 2, 2012, at 14:37 , Jan Lehnardt <jan@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 <bjnortier@gmail.com>
> >>>> Authored: Fri Sep 21 16:46:46 2012 +0100
> >>>> Committer: Jan Lehnardt <jan@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, []).
> >>>>
> >>>
> >>
> >>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message