+1 to squash. Although occasionally there are exceptions: a contribution might contain multiple features that can be broken out into separate commits, each with its own JIRA case.
Ensuring a clean, literate change history is one of the key responsibilities of committers. +1 to avoiding merges (i.e. git commits with more than one parent). Julian > On Feb 9, 2017, at 10:10 AM, Josh Elser <[email protected]> wrote: > > 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<[email protected]> 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<[email protected]> wrote: >>>> >>>> Our current policy is that we accept patches attached to JIRA case and >>>> pull requests to >>>> https://github.com/apache/calcite<https://github.com/apache/calcite>. 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 when 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 a 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, I >>>> did “git status” and I missed the 2 new files among all of the “.orig” and >>>> “.rej” files in my sandbox. >>>> >>>> In summary. I propose that we accept contributions only as pull requests >>>> to https://github.com/apache/calcite<https://github.com/apache/calcite>. >>>> 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 >>>> >>
