Hello, Jan. On Tue, Jun 30, 2015 at 05:02:54PM +0200, Jan Kara wrote: > BTW, I'd prefer if this was merged with the following patch. I was > wondering for a while about the condition at the beginning of > blk_clear_congested() only to learn it gets modified to the one I'd expect > in the following patch :) The patches are already merged, it's a bit too late to discuss but I usually try to keep each step quite granular. e.g. I try hard to avoid combining code relocation / restructuring with actual functional changes so that the code change A -> B -> C where B is functionally identical to A and C is different from B only where the actual functional changes occur. I think your argument is that as C is the final form, introducing B is actually harder for reviewing. I have to disagree with that pretty strongly. When you only think about the functional transformations A -> C might seem easier but given that we also want to verify the changes - both during development and review - it's far more beneficial to go through the intermediate stage as that isolates functional changes from mere code transformation. Another thing to consider is that there's a difference when one is reviewing a patch series as a whole tracking the development of big picture and later when somebody tries to debug or bisect a bug the patchset introduces. At that point, the general larger flow isn't really in the picture and combining structural and functional changes may make understanding what's going on significantly harder in addition to making such errors more likely and less detectable in the first place. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
