NagyDonat wrote:

> If I understand correctly, many of these commits overlap. So we wouldn't 
> really gain much by landing these one-by-one. While I'd be tempted to say to 
> land them commit by commit, it's probably easier if we do a thorough review 
> on this 5k+ diff and merge it in one go - while having a detailed squash 
> message.
> 
> What's your opinion @NagyDonat?

I admit that this commit is bigger than what I expected, especially if I want 
to read it line-by-line.

I feel that landing ~40 commits individually would be definitely too much, but 
perhaps if there is some clear logical separation, then we could split this 
into 3-4-5-6 PRs that are a bit smaller.

However, there are also advantages in squashing this large PR into a single 
commit – obviously, it is less work, and moreover that would have the advantage 
that it would collect all the changed lines into a single logical unit. (Of 
course commit messages like "Refactoring... Part 3/5" also create such 
connections, but those are softer and harder to follow.)

-------

@steakhal In my inline comments I suggested many places where I think that this 
PR should capitalize variable names in trivial methods (1-4 lines) where most 
of those lines are already touched by this PR. I feel that this is a unique 
opportunity – we probably won't touch those methods in the next 10 years – but 
I know that adding more complexity to an already big PR is a bit problematic. 
What do you think about this?

https://github.com/llvm/llvm-project/pull/198211
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to