ChuanqiXu9 wrote: > > > > > > While I may not able to look into them in detail recently, it may > > > > > > be helpful to split this into seperate patches to review and to > > > > > > land. > > > > > > > > > > > > > > > I initially considered this, but @vgvassilev said in > > > > > [root-project/root#17722 > > > > > (comment)](https://github.com/root-project/root/pull/17722#issuecomment-2706555950) > > > > > he prefers a single PR, also for external testing. > > > > > > > > > > > > Maybe you can test it with this and land it with different patches. So > > > > that we can revert one of them if either of them are problematic but > > > > other parts are fine. > > > > > > > > > This is a relatively small patch focused on reducing the round trips to > > > modules deserialization. I see this as an atomic change that if it goes > > > in partially would defeat its purpose. What's the goal of a partial > > > optimization? > > > > > > I think partial optimizations are optimization too. If these codes are not > > dependent on each other, it should be better to split them. > > Given the scale of the patch, it may not be serious problem actually. I > > still think it is better to land them separately, but if you want to save > > some typings. I don't feel too bad. > > Honestly I am more concerned about the tests that @ilya-biryukov is running. > As long as they are happy I do not particularly care about commit style. > Although it'd be weird to land 40 line patch in many commits :)
I don't feel odd. I remember it is (or was) LLVM's policy that smaller patches are preferred : ) https://github.com/llvm/llvm-project/pull/133057 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
