Re: [PATCH v5 13/16] export_report: Rehabilitate script

[Luis Chamberlain]

On Fri, Oct 11, 2024 at 03:22:54PM -0700, Matthew Maurer wrote:
> On Fri, Oct 11, 2024 at 3:13 PM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote:
> >
> > On Wed, Sep 25, 2024 at 11:38:28PM +0000, Matthew Maurer wrote:
> > > * modules.order has .o files when in a build dir, support this
> >
> > The commit log is not clear, is it that it's always had *.o files, and
> > you're adding them now, why? Why is the .ko search now removed?
> 
> The script was broken when I found it,

My point is that commit messages should describe all this.

> but it was a script that
> analyzed MODVERSIONS, so I tried to ensure it would still work with my
> changes.

OK yes please add this informaiton into the comit log.

Also expand on it as to to *why* this was not a critical issue.

> This necessitated rehabilitating it first. I did not touch
> `.modules.order` files, but they contained `.o` and so this script
> wouldn't run correctly.

Just elaborate on the commit log. We cannot guess what is happening, and
so bringing clarity into exactly what you are doing and *why* is helpful
to determine the impact of not applying this.

> > > * .mod.c source layout has changed,
> >
> > When, why did this change not happen at that time?
> 
> It was changed for readability [1] back in 2019. I assume the change
> did not happen at that time because this script is rarely run.

This is crucial information to include in the commit log. When *would*
you use it? So that use case was broken since 2019? What negative
effects has this implicated?

> If we'd
> prefer to discard this patch and ignore the script instead (or remove
> it?), that's fine.

The feedback is for you to improve the commit logs, not to tell you to
not do something. We cannot read your mind, and this smelled like a fix,
but without a true evaluation and documentation of the impact of not
having this in place. These things need to be thought out and written
in the comit log.

> 
> [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@xxxxxxxxxxxxx/
> 
> >
> > > update regexes to match
> >
> > Why did this not break anything before ? Is this fixing something, or
> > is it prep work?
> >
> > > * Add a stage 3, to be more robust against additional .mod.c content
> >
> > Future .mod.c changes?
> 
> The rest of this series adds additional `.mod.c` content to support
> the string names. This stage 3 is intended to prevent that from
> causing the script to choke.

The commit log should clarify all this.

  Luis