Hi Sean, On Sat, 21 Mar 2020 at 12:57, Sean Anderson <[email protected]> wrote: > > On 3/21/20 10:43 AM, Simon Glass wrote: > > Hi Sean, > > > > On Thu, 19 Mar 2020 at 23:37, Sean Anderson <[email protected]> wrote: > >> > >> By default patman generates a combined changelog for the cover letter. This > >> may not always be desireable. > >> > >> Many patches may have the same changes. These can be coalesced with > >> "Series-process-log: uniq", but this is imperfect. First, this cannot be > >> used when there are multi-line changes. In addition, similar changes like > > > > We could perhaps support this if we used indentation to indicate multiple > > lines > > > > - line 1 of a multi-line message > > line 2 here > > - this is line 1 of a single-line message > > That is probably the best solution in general.
OK then let's do that. > > >> "Move foo to patch 7" will not be merged with the similar "Move foo to this > >> patch from patch 6". > >> > >> Changes may not make sens outside of the patch they are written for. For > > > > sense > > > >> example, a change line of "Add check for bar" does not make sense outside > >> of the context in which bar might be checked for. Some changes like "New" > >> or "Lint" may be repeated many times throughout different change logs, but > >> carry no useful information in a summary. > >> > >> Lastly, I like to summarize the broad strokes of the changes I have made in > >> the cover letter, while documenting all the details in the appropriate > >> patches. I think this make it easier to get a good feel for what has > >> changed, without making it difficult to wade through every change in the > >> whole series. > >> > >> For these reasons, this patch adds an option to disable the automatic > >> changelog. > >> > >> Signed-off-by: Sean Anderson <[email protected]> > >> --- > >> > >> tools/patman/func_test.py | 2 +- > >> tools/patman/patchstream.py | 7 ++++--- > >> tools/patman/patman.py | 6 +++++- > >> 3 files changed, 10 insertions(+), 5 deletions(-) > > > > Thanks for the great explanations on your patches. > > > > In the case where there is no automatic change log on the cover > > letter, do you use 'Series-notes' instead? I'd like to enforce some > > sort of change log in the cover letter. > > That could work, but it doesn't really support combining changes from > multiple patches. I've been doing it manually, but some automation would > definitely be better. One idea would be to do something like It does collate the notes and put it in the cover letter, or at least it should. > > Series-changes 12: > * Summary of the below changes > - Small change > - Move foo to bar.c > - Reformat code to do baz > * Fix bug where device caught fire > > Where only lines beginning with '*' would be included in the combined > changelog. The character could be configurable. Another option could be > to do something like > > Series-changes 12: > - Summary of the below changes > - Small change > - Move foo to bar.c > - Reformat code to do baz > - Fix bug where device caught fire > > This is nicer aesthetically, but would make any future multi-line > treatment of series-changes more difficult. Perhaps the clearest thing is to have: Series-changes: (behaviour as now) Commit-changes: (change log just in the commit/patch, not repeated in the cover letter) Cover-changes: (change log just in the cover letter) This should minimise duplication and makes the scheme optional. > > One last option could be to do something like > > Summary-changes 12: > - Summary of below changes > - Fix bug where device caught fire > > Where those changes would be included in the cover letter, but not > Series-changes. This is probably the easiest to implement, but risks > adding duplication and making commits more verbose. > > > I also think this would be better as a tag in a commit, like > > 'Series-no-change-log: yes'. That way you set it up when you create > > the patches, and it persists without needing to add the options each > > time. > > That's probably the best approach. > > Should I rebase this series on top of the patch you cc-d me on ("patman: > Update to use absolute imports")? Can we hold off until we figure out what we definitely want? Regards, Simon
