[PATCH 2/2] patman: Add option to disable combined changelogs

[Simon Glass]

Hi Sean,

On Sat, 21 Mar 2020 at 12:57, Sean Anderson <seanga2 at gmail.com> wrote:
>
> On 3/21/20 10:43 AM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Thu, 19 Mar 2020 at 23:37, Sean Anderson <seanga2 at gmail.com> 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 <seanga2 at gmail.com>
> >> ---
> >>
> >>  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