On Thu, Mar 5, 2020 at 7:58 PM Abdelatif Guettouche <abdelatif.guettou...@gmail.com> wrote: > > > 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.
Yes, the contributor should make sure the patch set is split/merged in a reasonable way before creating PR. If not, maintainter could discuss the issue in the PR and let contributor update PR again.This is better than maintainer directly squash the patch set into mainline, because we need teach the new member what's the best practice the community like to follow and avoid they make the same mistake in the new 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? > > > >
