Basile Starynkevitch <bas...@starynkevitch.net> writes: > On Tue, 2010-08-24 at 15:19 -0700, Ian Lance Taylor wrote: >> >> > However, our patch also added some improvements to gengtype itself >> >> Improvements are desirable, but if it is all possible you should >> separate these improvements from your other work. It is very hard to >> review patches which combine different unrelated ideas in a single diff. > > > We only improved some minor points of gengtype when we had bugs related > to code that we found difficult to understand or to read. In particular > > * replacement of the kludge of putting a language bitmask four bytes > before the path name of input files with a real structure for input > files. > > * replacement of the get_output_file_with_visibility code with a > sequence of regular-expression rules. > > None of these patch is unrelated to our main work of adding persistency > to gengtype. We had to make these improvements to make the whole thing > work.
That is fine, but I am going to reiterate: from your description, those changes can be separated from the rest of the patches. Therefore, they should be sent in separately. In order to get a patch accepted, your goal should always be to provide minimal isolated changes that are easy to understand. The larger the patch, the harder it is to understand and review. Always make changes as small as possible. Separate cleanups from substantive changes. Ian
