Agree! If you read the work flow I suggested you will see it is a rebase until review WF. Noise should always be squashed.
-----Original Message----- From: Abdelatif Guettouche [mailto:abdelatif.guettou...@gmail.com] Sent: Thursday, March 05, 2020 4:53 AM To: dev@nuttx.apache.org Subject: Re: squashing commits or not > Because if > the contributor take the time to split the change into serveral small > patch, which mean it's valuable to do so. I agree, but this isn't always the case. It is okay to have PRs with multiple commits, as you mentioned, we can even request to do so. However, some of the commits are just a sequence of the same change. That's valuable for a WIP, but IMO should've been squashed before creating the PR. On Thu, Mar 5, 2020 at 11:05 AM Xiang Xiao <xiaoxiang781...@gmail.com> wrote: > > On Thu, Mar 5, 2020 at 5:10 PM Abdelatif Guettouche > <abdelatif.guettou...@gmail.com> wrote: > > > > Hi, > > > > We do not squash commits unless they are related (Which should have been > > done by the OP in the firsr place). > > But even the patch related each other, it's better to keep the > individual small patch instead merging into one big patch. Because if > the contributor take the time to split the change into serveral small > patch, which mean it's valuable to do so. > > it is important to keep the patch as it without modification, if the > PR doesn't looks good, we can: > 1.Suggest the contributor modify the change and update PR again > 2.Or merge the PR and create a new PR fix the remaining issue. > Of course, the second approach should be selected only for the minor > issue(coding style, naming etc). Each individual patch should pass the > build and no regression first. > > > We actually encourage putting unrelated changes in separate commits. > > > > Most people send PR contain the multiple commit just because all of > them is related each other. If not, maintainer can request the > contributor split PR into small ones. > > > > > > > On Thu, Mar 5, 2020, 05:26 Takashi Yamamoto > > <yamam...@midokura.com.invalid> > > wrote: > > > > > hi, > > > > > > it seems that in nuttx it's common to squash commits when merging pull > > > requests. > > > > > > i'd like to suggest _not_ to do that because: > > > * it makes cherry-pick/backport/revert cumbersome > > > * larger changes are more difficult to read/understand > > > > > > on the other hand, i can think of only little benefit. > > > * aesthetic reasons? > > > * marginally save the repo size growth? > > > > > > how do you think? > > >
