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 <[email protected]> wrote:

>
> On Nov 2, 2012, at 14:41 , Robert Newson <[email protected]> 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 <[email protected]> wrote:
> >
> >>
> >> On Nov 2, 2012, at 14:37 , Jan Lehnardt <[email protected]> wrote:
> >>
> >>>
> >>> On Nov 2, 2012, at 14:02 , [email protected] 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 <[email protected]>
> >>>> Authored: Fri Sep 21 16:46:46 2012 +0100
> >>>> Committer: Jan Lehnardt <[email protected]>
> >>>> 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, []).
> >>>>
> >>>
> >>
> >>
>
>

Reply via email to