Re: [january-dev] Squash and Merge vs Create a Merge Commit

[Jonah Graham <jonah@xxxxxxxxxxxxxxxx>]

Hi Matthew,

Yes! In particular this is the key:

Developer gets the pull request clean, which means one or more
commits, where each commit is a suitable size. If a commit has an
error (eg a spelling error in a comment), the original commit should
be fixed up and replaced; the correction should not be a separate
commit. There MUST NOT be any commit in the chain where master is
merged into the pull request - that's what rebase is for.


At the moment most of the PRs in January are indeed a single commit.

I would still recommend using Squash and Commit if possible because it
adds to PR# to the commit message. TBH I have spent a long time in
Gerrit now and I just want all those features in GitHub, like
Change-Ids and cherry-picking between branches.

Jonah




~~~
Jonah Graham
Kichwa Coders Ltd.
www.kichwacoders.com


On 15 March 2017 at 10:03, Matthew Webber <matthew@xxxxxxxxxxxxxx> wrote:
> 100% agree of the unhelpful nature of the repo history.
>
> Personally, I don't like the "squash when accepting pull request" option. My
> feeling is that any squashing should be done by the pull request author,
> rather than the person accepting the pull request making their own guess as
> to what the correct level of granularity for the commits is.
>
> If the pull request looks like all or some or it should have been squashed,
> that should be referred back to the author (the first time, authors may need
> some guidance about how to proceed).
>
> Options like squash on accept that only need to exist because:
>
> Not all developers are aware of git squash.
> Not all developers are aware of git rebase.
> Not all developers are aware of git amend commit.
> Not all developers are aware that if you force push your change branch,
> GitHub notices that and correctly treats it as a new version of your pull
> request (the equivalent of a new patchset in Gerrit).
> And worst of all, some developers clearly never look at what they have
> actually pushed.
>
> I don't expect people to become experts in git as well as everything else.
> However, reviews should identify when the pull request is poorly
> constructed, and that's a good time for a developer to learn how to clean it
> up.
>
> My ideal workflow would be:
> (1) Developer gets the pull request clean, which means one or more commits,
> where each commit is a suitable size. If a commit has an error (eg a
> spelling error in a comment), the original commit should be fixed up and
> replaced; the correction should not be a separate commit. There MUST NOT be
> any commit in the chain where master is merged into the pull request -
> that's what rebase is for.
> (2) If the clean pull request is a single commit, then that can be rebased
> master, and then fast forward.
> (3) If the clean pull request is a multiple commits, then it's a feature
> branch, and it can be rebased on master, and then merge (don't FF at this
> point, even though it is possible, but use a merge so that the series of
> commits for the feature is preserved).
>
> If you don't like the command line, EGit can do all this.
>
> FWIW!
>
> Matthew
>
> On 13 March 2017 at 19:09, Jonah Graham <jonah@xxxxxxxxxxxxxxxx> wrote:
>>
>> Hi devs,
>>
>> At the moment we are adding to the git history lots of irrelevant commits.
>> Nearly half of the commits are merge commits that just have the effect of
>> making our history messy looking without adding much information.
>>
>
>
> _______________________________________________
> january-dev mailing list
> january-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from
> this list, visit
> https://dev.eclipse.org/mailman/listinfo/january-dev
>