[cfe-dev] [llvm-dev] [lldb-dev] GitHub anyone?

Matthias Braun via cfe-dev cfe-dev at lists.llvm.org
Mon Jun 6 14:17:45 PDT 2016


> On Jun 6, 2016, at 1:20 PM, Craig, Ben via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> 
> On 6/6/2016 2:51 PM, Bruce Hoult via cfe-dev wrote:
>> - developer commits locally to a feature/bug release branch. Tidy up into a small number of logical commits, nice messages etc. I'd suggest it's good to have at least two commits: 1) a commit adding a test that fails, and is marked as expected to fail, demonstrating the bug or lack of feature. 2) a commit that fixes the bug or adds the feature, and marks the test as expected to pass.
> The previous statement (sort-of) contradicts the following statement:
>> That gives a master history with exactly one commit per feature or bug fix.
> In the master history, I would prefer the test and associated code change to be in the same commit.  I will suggest my own workflow.  I will note that others have already voiced their disapproval of the squash commits that you will see.
> 
> - developer commits locally to a feature/bug dev branch. You can commit work in progress, experiments, have bad commit messages etc
> 
> - developer pushes their feature/bug dev branch to their github fork of llvm, issues a pull request
> 
> - the appropriate maintainer (or or automatic system) causes build and tests to be run on the proposed bug fix.
> 
> - if the tests work, then do a "git merge --squash" to master.  TBD: which commit message(s) should be used?  First commit? Last commit? Pull Request description?
This point is where I don't understand why we need the reviewer or the system to do the squashing. I think it is fair to ask the developer to do the squashing before submitting the patch (he can still maintain a non-squashed branch on his local machine). After all if we do not want all the intermediate and fixup commits in the final history, why would the reviewer want to read them?

- Matthias


More information about the cfe-dev mailing list