kriegaex commented on PR #124: URL: https://github.com/apache/xalan-java/pull/124#issuecomment-1817328267
> There are two issues here: > > 1. Certain projects (Xalan included) lack review power, commit power, and code quality at the same time. It forces contributors to run into "multiple depending PRs" or "wait N+1 years in-between the PRs so they are merged". > ^^ I am not sure it is something that can be fixed in Xalan. There are virtually no active committers, committers rarely review others code, and the code quality is wonderful. While I agree that resource starvation is a common problem in many OSS projects, it is not my point here. Even if in a very actively managed project someone reviews PRs on a daily basis, a contributor might,just like I did, work on issue A, noticing B and prepare multiple PRs in short order, e.g. the same day. Rebasing PRs is always a requirement, and it is eased by rebasing on a commit history without merge commits, hence the idea to fast-forward them. > 2. Sure it was a bad idea to squash #122 into a single commit. +100500 to what you say. Thanks for supporting me in this regard. > > In any case: Normally I'd only squish upon merge into the main stream. Which should normally be after regression testing has been passed, unlike the individual commits > > @jkesselm , please re-read what @kriegaex says. There are several very valid reasons to keep commits like in #122 separate, and I 100% agree with @kriegaex that structuring PR122 as five independent commits, and merging it as 5 independent commits was the best idea possible. @kubycsolutions / @jkesselm (which of the two user names do you prefer to me mentioned by?), Vladimir understood correctly. I suggested to **never** force-squash PRs into single commits, because in most cases users would bisect problems in a release version, not in a random development version. Releases, however, are built from the main branch, and there again would be the clunky squashed commits. Vladimir explained it very well in his example use case. > I never cared reviewing Maven scripts as those are a waste of time. Sorry to ask off topic, but as you brought it up: Why would reviewing Maven scripts be a waste of time? They influence, even define the whole build process. Any change can mess up the build. Reviewing them is well invested time. > Of course, having commits separate would help review for those who care to review, however, it was not the sole motivation. Yes. The main motivation should be separation of concerns. Imagine you mix functional changes with structural refactoring and major reformatting. Now you make one big commit. When the next developer or user scrolls through a `git blame|annotate` version of a file, she will probably see the commit comment concerning the functional change in all the lines that were only reformatted. She would be confused, wondering how those lines are related to the description in the commit message. I could mention more examples, e.g. mixing different types of unrelated functional changes - you get the picture. 😉 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
