Hello Jacques,

"[email protected]" <[email protected]> writes:

> This is in relation with "Github PRs and Jira" thread 
> (https://markmail.org/message/s422zuivbmzglph4)
>
> Below is my 1st experience of GitHub PR merge in the context of ASF dual 
> hosting with GitBox. So I somehow made a jump in the void...
>
> After reviewing the change I picked "squash and merge PR" among the 3
> GitHub PR merge options. Because I did not want to break the linear
> Git flow, it seemed the best option. But despite "squashing" we got  3
> commits; 2 from Daniel Watford and one from I. I don't know if it's
> possible to do otherwise with the 2 other options. Maybe it's good to
> know that Daniel contributed...
>
> Also I expected that my comment in GitHub would be used as the final
> commit comment. So I then tried to amend the commit by modifying ONLY
> the comment, and then push comment after a pull. But I got a
> fast-forward issue and after trying several pull and push tricks I
> understood I'll never pass over this issue:
>
>    "[remote rejected]       trunk -> trunk (pre-receive hook declined)"
>
> Because it's related to having "receive.denynonfastforwards" set to
> true on GitBox server[1] and I don't want to annoy Infra to temporary
> set it true for a such change.

One must never, never, never rewrite history of the trunk branch. :-)

> But I really wanted to change the commit comment to follow our
> template. So I decided to pull rebase, slightly change the
> ComponentContainerTest class, commit and push. Hence I created a new
> commit, somehow a duplicate of the PR merge, at least with a correctly
> formatted comment.
>
> This was an awkward experience, I don't know if i could have done it better, 
> ideas, advices?
> Maybe advising contributors to use our template in commit comments used by 
> GitHub PRs?

The best option I think is to clone the branch locally, amend the
necessary commits, push on the PR branch ask for validation from the
original author, and then from your terminal rebase on trunk and
fast-forward merge the commits (meaning without a merge commit).

Basically the better option to use Github Web interface only for
discussion/review.

> Also we can ask our contributors to use in the PRs title the format of
> our commit template. For instance in[2] instead of
>
>    OFBIZ-11331: Allow ComponentContainerTest to run on windows
>    rather
>    Fixed: Allow ComponentContainerTest to run on windows (OFBIZ-11331)
>
>    Then the commit/s would contain the explanations.
>
> I just fear we will have sometimes to explain, and ask contributors to edit 
> their comments or titles...
>
> I'll sleep on it and wait for comments before doing any additions or
> changes in our documentation about that, in relation with the "Github
> PRs and Jira" thread

I think it would be better to not use the PR feature as long as there is
not a community consensus that this is an acceptable contribution
channel.  As stated previously I am not against adopting Github PR *if
and only if* we have a plan to move out of Jira.

Given the pile of existing policies, I don't think this is a good idea
to simply provide more contribution options/policies, I think it is
important to review existing ones and consider replacing some of them.

For example we could deprecate patch based contribution, or at least
adopt the ‘git format-patch’ conventions.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

Reply via email to