+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 > > >
