Unfortunately there has been significant push back for small iterative PR's and as a result they have grown substantially involving multiple contributors, multiple commits, sometimes multiple branches to be worked on.
I fully agree and support all points that Jin raised: 1) Most contributions should be broken down into smallest possible PR's. 2) If a PR is small enough a single person can complete it. 3) If a majority of PR is done by someone, and there's some minor issue he/she needs help with it could be done in a follow up PR by anybody, including the reviewer. Best regards, Anton чт, 12 июл. 2018 г. в 10:11, Hao Jin <[email protected]>: > +1 for Marco's proposal on using the co-author field. IMHO the usage of > co-author field should not be that often, consider: > 1) If a PR is so big that it needs multiple people to contribute a > substantial part of it, why can't that PR be broken down into smaller PRs > each submitted by single one of them? > 2) If a PR is small enough (for example, < 300 lines), why can't a single > person complete it? > 3) If a majority of PR is done by someone, and there's some minor issue > he/she needs help with(a small bug, incomplete doc), why can't that be done > through code reviews? > From the above 3 situations we can see that collaborations on individual > PRs should not be quite often, but I do agree that it could still be > necessary when someone lacks the related expertise/knowledge on some > necessary part of that PR given that the required knowledge cannot be > picked up in a short period of time. > > I do agree that keeping the commit histories of PRs clean is very important > as it could be confusing when reviewing PRs, but that really depends on > personal preferences (For my case, I usually do "git commit --amend" on > trivial changes and get a new commit for bigger changes such as a > checkpoint of my whole PR). With growing the community and attracting more > contributors as a high priority for our project, I would prefer that we do > not put even more burden on the contributors by asking them to manage and > squash the commits themselves just for the not-so-often cases of having > multiple contributors on a single PR. > Best regards, > Hao > > On Thu, Jul 12, 2018 at 12:35 AM, Marco de Abreu < > [email protected]> wrote: > > > Hi Naveen, > > > > I'm in favour of the squashing, considering the number of commits in some > > PRs and especially because of some people making commit messages a la > "fix" > > "fix" "fix" all the time. Additionally, it gets hard (not impossible, > just > > more inconvenient) to determine the atomic states of master - aka, which > > commits are separate from each other. You should consider that > intermediary > > commits are unstable (fail CI) and thus it could be very hard to bisect > > failures in future - and the commit history gets cluttered. > > > > As alternative, I'd like to suggest the co-author field for these cases. > > Further documentation is available at > > https://blog.github.com/2018-01-29-commit-together-with-co-authors/. > > > > I definitely agree with the second part. We should all lead by example > and > > maintain a high quality by keeping our commit messages clean and > > meaningful. When I receive an email notification that a new commit has > been > > added and it only contains "fix" as title, it's not that helpful and also > > it's hard to track the development of a PR overtime. E.g., why has > > something been changed? Was there maybe a bug that we didn't cover with > > tests but the author just hacked something to get it to work but the > > problem still lays somewhere? We won't know that way and it makes it > harder > > for us to review. > > > > Best regards, > > Marco > > > > > > Naveen Swamy <[email protected]> schrieb am Do., 12. Juli 2018, 10:09: > > > > > Hi All, > > > > > > I am seeing that maintainers merge PRs into the repo, they are > squashing > > > the commits in the PR, which I understand and agree is to keep a sane > > > commit history, however this is causing problem when there are multiple > > > contributors involved on a PR(by contributing to a fork of the repo) > this > > > effectively removes credit for multiple contributors involved and shows > > all > > > code as authored by the contributor who created the PR. > > > > > > Can I request maintainers to not squash PRs if there are multiple > > > contributors involved on the PR. > > > > > > Also on the same note, I request contributors(regardless of multiple > > > contributors or not) to keep a clean commit history by squashing the > > > commits and not push all your WIP commits to the PR. this will help us > > keep > > > our commit history clean and meaningful. > > > > > > Let me know your thoughts/better approach or If I have misunderstood > how > > > this works. > > > > > > Thanks, Naveen > > > > > >
