René Scharfe <l....@web.de> writes: > It works best for changes whose effects are constrained to within the > affected functions, but have crucial information located outside the > three default lines of context. An example would be a change at the end > of a function for which a reviewer might need to know the type of some > variables declared at the top.
Yup. >> If there were topics in flight that touch any of these include >> blocks, the patch would not apply and a reviewer who is interested >> in these fixes ends up needing to wiggle them in somehow. > > Instructing git am or apply to ignore extra context lines using -C3 > or similar would help in such a case. It may, but that is still extra work ;-) > This one here requires one more piece of information, though, namely our > convention of wrapping header files in guard defines to make repeated > includes of them no-ops. We do that for those removed by the patch, but > we have a few exceptions to that rule in our repo (at least > command-list.h, kwset.h, sha1dc_git.h, tar.h, unicode-width.h). So in > that sense it's not such a good example of a self-sufficient patch. :) Not really. "We use header guards" is an argument that demotes this cleanup from "must have" to "nice to have". If a project did not use header guards or including the same header twice were an error, the patch in question would have been more necessary, but that wouldn't have changed the correctness of the patch, I think.
