couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Shorin <kxe...@gmail.com>
Subject Re: [DISCUSSION] GitHub PRs merging policy
Date Mon, 23 Mar 2015 11:28:05 GMT
Hi Garren,

I cannot say that is your workflow is wrong neither I cannot say the
mine is not. I'm more interested to define the one with which everyone
(or at least majority) is happy (:

The default merge behaviour is fast forward, but it fallbacks to no-ff
if git cannot perform it or you're merges a tag. So it can have a
problem of p.2 when information about a committer is lost.

P.S. Thanks for sharing gist! Very useful.

--
,,,^..^,,,


On Mon, Mar 23, 2015 at 11:27 AM, Garren Smith <garren@apache.org> wrote:
> Here is how the Fauxton team merge our commits. We probably don’t follow the most scientific
method but it seems to work well enough.
>
> 1) Add ability to remote fetch PR’s https://gist.github.com/piscisaureus/3342247 <https://gist.github.com/piscisaureus/3342247>
instead of having to add the contributor each time
> 2) git fetch (fetches all PR’s)
> 3) git merge pr/111
> 4) git push
>
> Using that gist is a real time saver. I think by default git merge is using git fast
forward, I’m not sure.
>
> I will now wait for Alexander’s email that tells me how wrong we are :)
>
> Cheers
> Garren
>
>
>> On 22 Mar 2015, at 10:02 PM, Alexander Shorin <kxepal@gmail.com> wrote:
>>
>> Hi devs,
>>
>> I'm deeply concerned about the way how we should handle pull requests
>> that comes on GitHub. Currently, I see the following ways to process
>> them:
>>
>> 1) Explicit merge aka GitHub workflow
>> git remote add contributor https://github.com/...
>> git fetch contributor
>> git merge --no-ff contributor/pr-branch
>>
>> Example: https://github.com/apache/couchdb-fabric/commit/a4d985252
>> Pros:
>> - Automatically closes PR on GitHub
>> - Strong reference to origin of incoming changes
>> - Responsible committer name is clearly defined in history
>> Cons:
>> - That's also the way to pollute commit history with useless merge
>> commits. No, they are useful, but not for a single commit changes.
>>
>> 2) Fast forward merge.  Same as 1) but without merge commit.
>>
>> Example: https://github.com/apache/couchdb-documentation/commit/27cc733
>> Pros:
>> - Clean history
>> - Automatically closes PR on GitHub
>> Cons:
>> - The related PR thread may contain some important information about
>> these changes and the backward reference becomes lost outside of
>> GitHub.
>> - There is no information about who did actually merged those changes
>>
>> 3) Apply patches manually
>> curl -O https://github.com/apache/couchdb-*/pulls/42.patch
>> Edit the patch to put magic "This closes #42" into commit message
>> git am --signoff < 42.patch
>>
>> Example: https://github.com/apache/couchdb-fabric/commit/adaf5c23
>> Pros:
>> - Clean history
>> - Responsible committer name is clearly defined in history
>> - Automatically closes PR on GitHub
>> Cons:
>> - Need to put PR number into commit message to make PR automagically
>> closed on merge
>> - Not suitable for large series of changes
>> - Literally closes PR on GitHub instead of merge (some people
>> concerned about that)
>>
>> I was used the latter one. Recently I'd tried the other ways. I'm not
>> happy with the results and I'm still not sure which way to use for
>> CouchDB.
>>
>> Personally, I tend to follow the first way when PR contains multiple
>> commits and the third when it's a question of some single change. But
>> would like to discuss the this workflow and make sure that mine is
>> fine for the other team.
>>
>> One day we'd tried to write our Git workflow, but it struggled because
>> of some details. Let's return to that idea and make it iterative, case
>> by case. Handling pull requests is a good topic for the start I think.
>>
>> Please share your though on this. Thanks!
>>
>> --
>> ,,,^..^,,,
>

Mime
View raw message