calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Mior <>
Subject Re: [DISCUSS] Patches
Date Thu, 09 Feb 2017 18:14:04 GMT
Agreed that rebasing is highly preferable. I'll note that GitHub has a
"rebase and merge" option on pull request pages

This may not always be advisable since it means tests were not run on the
resulting tree. However, this could be handy in the case where the
committer is confident the result will be safe and it saves time asking the
contributor to do the rebase.

Michael Mior

2017-02-09 13:10 GMT-05:00 Josh Elser <>:

> Since we're discussing it, this was the only consideration I had in moving
> to PR's:
> * Strongly suggest that a PR is applied as one commit (rebased by the
> contributor or developer applying the change). Avoiding the word "must"
> because there are edge-cases where multiple commits would be better
> * Developers must rebase a change (avoid the merge commit) so the merge is
> always a fast-forward merge.
> The lineage just gets so hairy for history if we start getting a bunch of
> branches/merges. This is what we've observed in PR's (at least, I have
> always seen this happen) -- if we are defining policy, it might be good to
> also codify this :)
> Julian Hyde wrote:
>> In principle it is hard to compute the delta of a pull request, but in
>> practice it is easy. A well-formed pull request is a branch that is a small
>> number of commits away from the master branch at the time, and the pull
>> requests that we tend to accept are well-formed.
>> Since we don’t rewrite the master branch, you can easily apply the pull
>> request using “git rebase”. Because git knows where where the pull
>> request’s branch meets the master branch, it can do a better job than
>> “patch” could.
>> Julian
>> On Feb 8, 2017, at 12:21 PM, Alan Gates<>  wrote:
>>> I agree that PRs are easier to manage than attaching patches to JIRA.
>>> And now days most contributors seem to prefer them as well.
>>> One question I have is about traceability and findability.  It is very
>>> nice for people to be able to come to JIRA and figure out if others have
>>> had the same problem they have, and if so if and where it's fixed, and
>>> exactly which commits they need to pick up if they want the fix.  Can all
>>> this be achieved with just PRs?
>>> If the answer is that PRs can't achieve that, I'd still vote for moving
>>> to them.  But I would also suggest continuing to open JIRAs that point to
>>> the PRs.
>>> Alan.
>>> On Feb 8, 2017, at 11:33 AM, Julian Hyde<>  wrote:
>>>> Our current policy is that we accept patches attached to JIRA case and
>>>> pull requests to<
>>>>>. I would like to propose that we no
>>>> longer support patches.
>>>> Why? I argue that it makes the process easier for the committer. The
>>>> pull request implicitly does “git add” and “git remove”, whereas
>>>> applying a patch you have to remember to apply these. The pull request
>>>> comes in a branch, so if I modify the code as I am reviewing it, I can
>>>> easily save and restore my state. Also, a pull request is “valid” as
>>>> contribution, from an IP standpoint, even when not accompanied by a JIRA
>>>> case.
>>>> Recently I went through 5 rounds of patches for a particular feature. I
>>>> couldn’t tell what had changed between one iteration of the patch and the
>>>> next (you can’t “diff" patches - you need to apply the patches to separate
>>>> git branches and diff the branches - yuck!). And I went through 3 test
>>>> cycles and 24 hours before I managed to “git add” all of the files. Yes,
>>>> did “git status” and I missed the 2 new files among all of the “.orig”
>>>> “.rej” files in my sandbox.
>>>> In summary. I propose that we accept contributions only as pull
>>>> requests to<
>>>>>. If they are non-trivial they
>>>> should be accompanied by a JIRA case. Committers can propose changes any
>>>> way they like, as long as they commit the changes themselves, but if they
>>>> want to make it easier for others to review, they should use either a
>>>> personal git branch or a pull request.
>>>> Julian

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