Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [smarthome-dev] Accept merge request using squash

Hi Markus,

Yes, the commit message should imho be edited (and that’s what I have always done so far).

Normally, we would ask the contributor to squash the commits himself (as we did in the past before the Github feature existed) and thus we were left with a single message and a single sign-off.

By using the Github feature, we only want to shortcut this process and imho the goal should be to have the same result in the end. So my approach here was to edit the message in a way that the contributor would have chosen after squashing. In most cases, this should simply be the very first commit message as normally most further commits are merely addressing review comments (and thus are relevant on their own, otherwise we rather do a rebase merge).

So in the example below, the "* fixed groovy tests“ is totally irrelevant here and the commit message should simply be:

> ===
> * removed getKey() method as it isn't needed anymore after introducing
> the Identifiable interface
> 
> Signed-off-by: Kai Kreuzer <kai@xxxxxxxxxxx>
> ===


Does that make sense?

Regards,
Kai



> On 3. Aug 2017, at 10:16, Markus Rathgeb <maggu2810@xxxxxxxxx> wrote:
> 
> Hello,
> 
> if we accept a pull request using the Github "merge & squash" feature,
> Github create one commit message that contains all the messages of the
> contained commits.
> 
> This results into a message that could contain multiple Signed-off-by entries.
> If we don't change it manually on the Web frontend this looks like
> e.g. this one: https://github.com/eclipse/smarthome/commit/9fe6be05a4dcc59117a9595bb888b7a1522290b4
> 
> Do we care about?
> Should we all the time edit the commit message so?
> For the given example above changing:
> 
> ===
> * removed getKey() method as it isn't needed anymore after introducing
> the Identifiable interface
> 
> Signed-off-by: Kai Kreuzer <kai@xxxxxxxxxxx>
> 
> * fixed groovy tests
> 
> Signed-off-by: Kai Kreuzer <kai@xxxxxxxxxxx>
> ===
> 
> to
> 
> ===
> 
> * removed getKey() method as it isn't needed anymore after introducing
> the Identifiable interface
> * fixed groovy tests
> 
> Signed-off-by: Kai Kreuzer <kai@xxxxxxxxxxx>
> ===
> 
> or sometimes we could the second message at all (for the example
> perhaps because fixing the test should be done in the first commit).
> 
> So, should the committer edit the commit message all the time if it is "useful"?
> 
> Best regards,
> Markus
> _______________________________________________
> smarthome-dev mailing list
> smarthome-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://dev.eclipse.org/mailman/listinfo/smarthome-dev



Back to the top