On Sat, Oct 3, 2020 at 12:19 PM David Hurka <david.hu...@mailbox.org> wrote: > > Why should colleagues navigate through any commits, when the MR is intended to > be squashed? Wouldn’t squash merge make it easier to review an MR? >
No because squashing happens only when merging, i.e. *after* reviewing. So if you review commit-by-commit you have more work and you're more likely to comment on things which turn out to be fixed by a later commit. > > I think this can not be expected from new contributors. > > If we reject a patch submitted a new user with “Thanks, your patch finally > fixes this annoying bug, but we can not accept it, because I don’t like your > git history. Please learn git first.”, that would IMHO be the first step to > make KDE a closed community. > I think that the other position is more accurately stated as follows: if a patch needs work on the git commit level but the code is fine, perhaps the maintainer should sit down and take time to checkout that branch locally, do their git magic until they are satisfied and then push that result to master. Then just leave a comment on the MR thanking people for their work, explaining their changes got merged into master via a different route and invite the contributor to test the new shiny to validate that their issue is fully fixed. A similar thing is also needed w.r.t. rebasing if for example you want to preserve a linear history in master: you can't expect drive by contributors to have infinite patience to check back and rebase, but it's not unreasonable for a maintainer who insists on linear history to shoulder that burden when they get round to merging things in. In general both approaches would point towards contributors not merging things on their own by default until they're familiar with the project and made a co-maintainer. It is also more closely aligned with the PR/MR workflow itself: note the R stands for "request" after all :) With active maintainers, that should not be hostile to new contributors, since you mostly care that your issue gets scratched and not who does the scratching... But it does require very active maintainers and it may not be a good fit for all projects. :) Regards, - Johan Ouwerkerk
