From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Mon, 4 May 2020 08:39:07 -0600
Subject: [PATCH v3 2/4] patman: Suppress empty changelog entries
In-Reply-To: <20200503215533.360196-3-seanga2@gmail.com>
References: <20200503215533.360196-1-seanga2@gmail.com>
 <20200503215533.360196-3-seanga2@gmail.com>
Message-ID: <CAPnjgZ1b31wKpf+8=DN1fTP=G7pZwa70GKYNzi8PApmsuzjhLA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Sean,

On Sun, 3 May 2020 at 15:55, Sean Anderson <seanga2@gmail.com> wrote:
>
> Patman outputs a line for every edition of the series in every patch,
> regardless of whether any changes were made. This can result in many
> redundant lines in patch changelogs, especially when a patch did not exist
> before a certain revision. For example, the existing behaviour could result
> in a changelog of
>
> Changes in v7: None
> Changes in v6: None
> Changes in v5:
> - Make a change
>
> Changes in v4: None
>
> Changes in v3:
> - New
>
> Changes in v2: None
>
> With this patch applied and with --no-empty-changes, the same patch would
> look like
>
> (no changes since v5)
>
> Changes in v5:
> - Make a change
>
> Changes in v3:
> - New
>
> This is entirely aesthetic, but I think it reduces clutter, especially for
> patches added later on in a series.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> Changes in v3:
> - Document empty changelog suppression in README
> - Fix KeyError when running tests
> - Fix no changes message being output for revision 1
> - Fix no changes message sometimes being output before every
>   non-newest-revision change
> - Make the newest_version logic more robust (and ugly)
> - Update commit subject
>
> Changes in v2:
> - Add a note when there are no changes in the current revision
> - Make this the default behaviour, and remove the option
>
>  tools/patman/README    | 21 ++++++++++++++++++++
>  tools/patman/series.py | 44 +++++++++++++++++++++++++++++++-----------
>  2 files changed, 54 insertions(+), 11 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Please see comment below though.

[..]

> diff --git a/tools/patman/series.py b/tools/patman/series.py
> index 6d9d48b123..4359442174 100644
> --- a/tools/patman/series.py
> +++ b/tools/patman/series.py
> @@ -146,38 +146,60 @@ class Series(dict):
>              Changes in v4:
>              - Jog the dial back closer to the widget
>
> -            Changes in v3: None
>              Changes in v2:
>              - Fix the widget
>              - Jog the dial
>
> -            etc.
> +            If there are no new changes in a patch, a note will be added
> +
> +            (no changes since v2)
> +
> +            Changes in v2:
> +            - Fix the widget
> +            - Jog the dial
>          """
> +        versions = sorted(self.changes, reverse=True)
> +        newest_version = 1
> +        try:
> +            newest_version = max(newest_version, int(self.version))
> +        except (ValueError, KeyError):
> +            pass
> +        try:
> +            newest_version = max(newest_version, versions[0])
> +        except IndexError:
> +            pass

Can we do this without exceptions so it is more deterministic?

E.g.

if 'version' in self:
   newest_version = max(newest_version, int(self.version))
if versions:
   newest_version = max(newest_version, versions[0])

> +
>          final = []
>          process_it = self.get('process_log', '').split(',')
>          process_it = [item.strip() for item in process_it]
>          need_blank = False
> -        for change in sorted(self.changes, reverse=True):
> +        for version in versions:
>              out = []
> -            for this_commit, text in self.changes[change]:
> +            for this_commit, text in self.changes[version]:
>                  if commit and this_commit != commit:
>                      continue
>                  if 'uniq' not in process_it or text not in out:
>                      out.append(text)
> -            line = 'Changes in v%d:' % change
> -            have_changes = len(out) > 0
>              if 'sort' in process_it:
>                  out = sorted(out)
> +            have_changes = len(out) > 0
> +            line = 'Changes in v%d:' % version
>              if have_changes:
>                  out.insert(0, line)
> -            else:
> -                out = [line + ' None']
> -            if need_blank:
> -                out.insert(0, '')
> +                if version < newest_version and len(final) == 0:
> +                    out.insert(0, '')
> +                    out.insert(0, '(no changes since v%d)' % version)
> +                    newest_version = 0
> +                # Only add a new line if we output something
> +                if need_blank:
> +                    out.insert(0, '')
>              final += out
>              need_blank = have_changes
> -        if self.changes:
> +
> +        if len(final) > 0:
>              final.append('')
> +        elif newest_version != 1:
> +            final = ['(no changes since v1)', '']
>          return final
>
>      def DoChecks(self):
> --
> 2.26.2
>

Regards,
Simon