Am 23/07/2024 um 16:57 schrieb Aaron Lauterer: > There are quite a few preparation changes in other sub-crates > (auto-installer, installer-common). > I've only gotten through them for now and haven't looked at the actual > post-hook crate stuff. > > Wouldn't it be nicer to split the preparation patches into their own > commit? It would make the patch smaller and the history would be a bit > clearer as we don't mix adding the post-hook crate with all the > preparation that is needed.
This isn't a binary thing, sometimes it can be nicer to have some preparatory stuff up-front, most often if it's not only affecting new code and code changes limited to the thing a patch wants to do, otherwise it can also be nice to see what is needed to make it work, especially as often preparatory patches cannot be reviewed in isolation anyway and one needs to look into the patches that they're preparing for anyway to actually be able to determine if the thing as a whole makes sense the way it's implemented, split and used. As mentioned, this is often subjective, both have some advantages and disadvantages, IMO the best heuristic is the mentioned "how much is existing code impacted and how much is this related to the semantic thing the patch wants to do", all else is a bit of a balance that different reviewers might also disagree a bit with, so it's also fine to reject a split or merge if you, as patch author, really think it would make it worse – ideally with some argumentation and avoiding hard lines to avoid "kerfuffle" or getting less reviews in the future ;-) unrelated and definitively not only applying just to you, only as a gentle reminder: I would like to see that replies on the mailing list get unrelated stuff trimmed out and use of inline/bottom posting, that makes it quicker to read and reply and also to digest (longer) threads. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
