Proposals for commit log policy

Proposals for commit log policy

[Alexander Amelkin]

[-- Attachment #1.1: Type: text/plain, Size: 3162 bytes --]

Hi Brad, all!

Before I even try to submit any changes directly to CONTRIBUTING.md, I'd like to discuss it here.

What I'm facing is the need to create Release Notes from time to time. Those Release Notes are for server admins, not for openbmc developers. The most difficult and time consuming parts about this effort are:

1. Figuring out the end user impact
2. Figuring out what changes have been brought in by "bump version" commits

Thankfully, for linux-obmc version bumps Patrick Williams and Joel Stanley do a great job by including the 'git shortlog' output. Unfortunately, not everyone is following this rule for bumps. I'd really like this to become a requirement for 'bump version' commits. They could then be parsed automatically. It would also be helpful if  the synopsis included the old and the new hash, e.g.:

=================
some-package: bump version 62286ef3 to 4fa63380

Author Name (2):
  commit 1 synopsis
  another commit synopsis

Another Author (3):
  great commit
  yet another great commit
  fix previous not-so-great commit 

Change-Id: some-long-gerrit-hash
Signed-off-by: The Version Bumper <bumper@nowhere.net>
=================

Including the component's origin git URL could also be helpful for automating change log fetching (finding that in the recipe is sometimes a non-trivial task for a script).

Now for item 1, it would be great if we could have a mandatory "End-user-impact" line/section, like these added to the following real commits:

=================
whitelist: Add new phosphor settings directories
    
The phosphor settings properties used to be stored under
/var/lib/obmc/ but they have moved to /var/lib/phosphor-*
directories.
    
Update the code update whitelist to include these
directories so that the settings properties and inventory
are preserved across code updates.
    
Closes openbmc/openbmc#3346
    
Tested: On Zaius, a PrepareForUpdate and an Apply preserve
the settings.

End-user-impact: None   
Change-Id: I4e528f90128e11af7d40537dceb2f9dd1db6cf73
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
=================

or:

=================
Fix to speed up boot to BMC Ready state
   
The legacy services(like system and chassis) was assigned nice value
of 19 to alleviate CPU stress at boot time. The systemd bootchart
showed that many services are dependedent on the system service. The
nice is set to default value of 0 and there is a consistent improvement
of ~10 sec in BMC getting to ready state.

End-user-impact: Speed up BMC booting to Ready state
Change-Id: I8f9f76da0a36c136243026aa7d7691e814634987
Signed-off-by: Tom Joseph <tomjoseph@in.ibm.com>
=================

What do you think? Is this feasible? Any other ideas how we could simplify creating release notes suitable for end users?

I totally understand that for the openbmc project the end users are system vendors, but for them (us) the end users are the server admins, and providing them with usable release notes is crucial. I'm quite sure that this is a kind of task that all vendors face, not just YADRO.

Alexander.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Proposals for commit log policy

[Gunnar Mills]

Hi Alexander,

On 8/21/2018 7:29 AM, Alexander Amelkin wrote:
> Thankfully, for linux-obmc version bumps Patrick Williams and Joel Stanley do a great job by including the 'git shortlog' output. Unfortunately, not everyone is following this rule for bumps.
The autobump tool is here: 
https://github.com/openbmc/openbmc-tools/blob/master/infra/autobump.pl
Could you enhance to have the ‘git shortlog’ in the commit message?
> Now for item 1, it would be great if we could have a mandatory "End-user-impact" line/section, like these added to the following real commits:
>
I really oppose another field/section in each commit. We already have a 
"tested" field. What happens when someone else wants a field for what 
systems this commit effects? This might be a bad example but hopefully, 
you see my point. :)

- Gunnar

> Alexander.

RE: Proposals for commit log policy

[Tanous, Ed]

I'm not really understanding the issue here.  Is the log information not available in the individual repositories?  Is it just hard to parse?  Could this be automated, either using a script, or a list of repositories?

While I think there's value in the logs, I don't think maintainers time is best used by creating git logs manually all the time, especially when it should be easy enough to parse the git logs between the recipe bumps.  I've had to do this before to do bisects before, and it's not too difficult once you get the hang of it.

I really do appreciate you generating a real set of release notes, and I want to make it easy, I'd just prefer if we as a project didn't move work onto the individual developers to make it happen.

-Ed

Re: Proposals for commit log policy

[Joel Stanley]

On Tue, 21 Aug 2018 at 22:01, Alexander Amelkin <a.amelkin@yadro.com> wrote:

> =================
> some-package: bump version 62286ef3 to 4fa63380
>
> Author Name (2):
>   commit 1 synopsis
>   another commit synopsis
>
> Another Author (3):
>   great commit
>   yet another great commit
>   fix previous not-so-great commit
>
> Change-Id: some-long-gerrit-hash
> Signed-off-by: The Version Bumper <bumper@nowhere.net>
> =================

> What do you think? Is this feasible? Any other ideas how we could simplify creating release notes suitable for end users?

I think this is a great suggestion. I would also like to see this happen.

Having the maintainers or authors of the packages submit their bumps
would ensure this does not become a burden for the central repository
maintainer.

An advantage of the maintainers of individual packages submitting
their changes is they know the best timing for integrating their
changes, and know what relevant details should be added to the bump
commit message.

Cheers,

Joel

Re: Proposals for commit log policy

[Alexander Amelkin]

[-- Attachment #1.1: Type: text/plain, Size: 1790 bytes --]


22.08.2018 18:01, Gunnar Mills wrote:
> Hi Alexander,
>
> On 8/21/2018 7:29 AM, Alexander Amelkin wrote:
>> Thankfully, for linux-obmc version bumps Patrick Williams and Joel Stanley do a great job by including the 'git shortlog' output. Unfortunately, not everyone is following this rule for bumps.
> The autobump tool is here: https://github.com/openbmc/openbmc-tools/blob/master/infra/autobump.pl
> Could you enhance to have the ‘git shortlog’ in the commit message?
Thanks for pointing me to it. I will enhance.

>> Now for item 1, it would be great if we could have a mandatory "End-user-impact" line/section, like these added to the following real commits:
>>
> I really oppose another field/section in each commit. We already have a "tested" field. What happens when someone else wants a field for what systems this commit effects? This might be a bad example but hopefully, you see my point. :)
I see your point, but I hope you feel my pain too. ;)
Technical description for developers is great... for developers. But is totally useless for end users.
What does an admin have to do with all those framework-related commits or dbus optimizations?
They want to know what has changed for them in the new release. And extracting that info from commit logs as they are now is painful and takes at least a leading software engineer. In my experience it takes about 3-4 days to "compile" the current commit logs between releases. That is, read them, comprehend them, ditch pure technical commit messages, translate commit messages that actually change the user experience from programmer's language to admin's language, and as a result of all that produce a document useful for admin end users. IMO, that's a waste of time and a very boring activity.

Alexander.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Proposals for commit log policy

[Alexander Amelkin]

[-- Attachment #1.1: Type: text/plain, Size: 2315 bytes --]

22.08.2018 23:39, Tanous, Ed wrote:
> I'm not really understanding the issue here.  Is the log information not available in the individual repositories?  Is it just hard to parse?  Could this be automated, either using a script, or a list of repositories?
You're obviously not getting my point. There are logs. But they are purely developer-oriented. Nobody actually cares (except for developers) that, for instance, some commit "Resolves some build failures and updates the bmcweb recipe to use the proper target names for image selection". That change is TOTALLY invisible to end users and must not get into Release Notes for a binary firmware release. Nobody actually wants to know that some changeset "replaces phosphor rest with bmcweb for the s2600wf target" or that "the handcoded harware sensor yaml file is merged with the final sensor yaml file". What end users want to know before they make a decision to upgrade is how the system behavior (user experience, responses to commands, etc.) has changed since the currently installed firmware. Writing end-user-centric Release Notes for a binary release takes a lot of time, because it requires analyzing all those purely developer-oriented logs. And that's a job that is done on a regular
basis by each vendor, I believe. And it takes, as I said, at least 3 full days contiguously of a qualified engineer. What I propose is ask each engineer who commits a change and knows his change, thus doesn't need to spend time analyzing it, to write a short description of end user impact. It will take like 5 extra minutes per commit.

> While I think there's value in the logs, I don't think maintainers time is best used by creating git logs manually all the time, especially when it should be easy enough to parse the git logs between the recipe bumps.  I've had to do this before to do bisects before, and it's not too difficult once you get the hang of it.
For bumps the task is "automatable" indeed. It's not actually as easy with Yocto as it may seem "to parse the git logs between the recipe bumps" (have you tried?), but it can be automated. But the end user impact information is not. It's either written by the developer in the commit log, or deduced (sometimes incorrectly or incompletely) by an engineer writing the Release Notes.

Alexander.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Proposals for commit log policy

[Brad Bishop]

> On Aug 22, 2018, at 7:58 PM, Joel Stanley <joel@jms.id.au> wrote:
> 
> On Tue, 21 Aug 2018 at 22:01, Alexander Amelkin <a.amelkin@yadro.com> wrote:
> 
>> =================
>> some-package: bump version 62286ef3 to 4fa63380
>> 
>> Author Name (2):
>>  commit 1 synopsis
>>  another commit synopsis
>> 
>> Another Author (3):
>>  great commit
>>  yet another great commit
>>  fix previous not-so-great commit
>> 
>> Change-Id: some-long-gerrit-hash
>> Signed-off-by: The Version Bumper <bumper@nowhere.net>
>> =================
> 
>> What do you think? Is this feasible? Any other ideas how we could simplify creating release notes suitable for end users?
> 
> I think this is a great suggestion. I would also like to see this happen.
> 
> Having the maintainers or authors of the packages submit their bumps
> would ensure this does not become a burden for the central repository
> maintainer.
> 
> An advantage of the maintainers of individual packages submitting
> their changes is they know the best timing for integrating their
> changes, and know what relevant details should be added to the bump
> commit message.

I also would like to see this.  To be clear I mean the commit msg text above,
and not a ‘Impact’ token.  

The autobump script was deployed at a time in the project when _every_ repository
had the same maintainer, so it made some sense back then.  Today we have a much
more distributed set of maintainers.

I’ve actually disabled the cron job that runs the script, because it needed
some rework to handle the new openbmc/openbmc repository layout.

I plan on simply _not_ turning it back on, and let maintainers do their own
bumps like Joel has been doing for the kernel moving forward (Thanks Joel).
Unless there are major objections.

thx - brad

> 
> Cheers,
> 
> Joel

Re: Proposals for commit log policy

[Andrew Geissler]

On Tue, Aug 28, 2018 at 5:30 AM Brad Bishop <bradleyb@fuzziesquirrel.com> wrote:
>
>
> > On Aug 22, 2018, at 7:58 PM, Joel Stanley <joel@jms.id.au> wrote:
> >
> > On Tue, 21 Aug 2018 at 22:01, Alexander Amelkin <a.amelkin@yadro.com> wrote:
> >
> >> =================
> >> some-package: bump version 62286ef3 to 4fa63380
> >>
> >> Author Name (2):
> >>  commit 1 synopsis
> >>  another commit synopsis
> >>
> >> Another Author (3):
> >>  great commit
> >>  yet another great commit
> >>  fix previous not-so-great commit
> >>
> >> Change-Id: some-long-gerrit-hash
> >> Signed-off-by: The Version Bumper <bumper@nowhere.net>
> >> =================
> >
> >> What do you think? Is this feasible? Any other ideas how we could simplify creating release notes suitable for end users?
> >
> > I think this is a great suggestion. I would also like to see this happen.
> >
> > Having the maintainers or authors of the packages submit their bumps
> > would ensure this does not become a burden for the central repository
> > maintainer.
> >
> > An advantage of the maintainers of individual packages submitting
> > their changes is they know the best timing for integrating their
> > changes, and know what relevant details should be added to the bump
> > commit message.
>
> I also would like to see this.  To be clear I mean the commit msg text above,
> and not a ‘Impact’ token.
>
> The autobump script was deployed at a time in the project when _every_ repository
> had the same maintainer, so it made some sense back then.  Today we have a much
> more distributed set of maintainers.
>
> I’ve actually disabled the cron job that runs the script, because it needed
> some rework to handle the new openbmc/openbmc repository layout.
>
> I plan on simply _not_ turning it back on, and let maintainers do their own
> bumps like Joel has been doing for the kernel moving forward (Thanks Joel).
> Unless there are major objections.

I object!  At least 90% of the auto bumps today go through without
issue.  This keeps our openbmc/openbmc master consistently up to date
which I think is a big benefit to our community.  We're able to
quickly find regressions with our automated tests which are constantly
running against the new content coming in. I feel like we're adding
more work with minimal benefit here.

To summarize:

- Adds a layer of manual work that has minimal benefits
- Prevents us getting small continuous changes, and instead will
encourage larger big chunks that are tougher to isolate fails on
- Even if individual repo owners are forced to do their own bumps, I
don't think many will address what started this thread, summaries of
the changes and impacts of each commit.

On the other side of thing I realize this is how yocto does it (manual
bumps) and it does allow more individual control for repo maintainers.
I just don't think at this point in our project, it's worth it.

Andrew

>
> thx - brad
>
> >
> > Cheers,
> >
> > Joel

Re: Proposals for commit log policy

[Patrick Venture]

On Tue, Aug 28, 2018 at 8:44 AM Andrew Geissler <geissonator@gmail.com> wrote:
>
> On Tue, Aug 28, 2018 at 5:30 AM Brad Bishop <bradleyb@fuzziesquirrel.com> wrote:
> >
> >
> > > On Aug 22, 2018, at 7:58 PM, Joel Stanley <joel@jms.id.au> wrote:
> > >
> > > On Tue, 21 Aug 2018 at 22:01, Alexander Amelkin <a.amelkin@yadro.com> wrote:
> > >
> > >> =================
> > >> some-package: bump version 62286ef3 to 4fa63380
> > >>
> > >> Author Name (2):
> > >>  commit 1 synopsis
> > >>  another commit synopsis
> > >>
> > >> Another Author (3):
> > >>  great commit
> > >>  yet another great commit
> > >>  fix previous not-so-great commit
> > >>
> > >> Change-Id: some-long-gerrit-hash
> > >> Signed-off-by: The Version Bumper <bumper@nowhere.net>
> > >> =================
> > >
> > >> What do you think? Is this feasible? Any other ideas how we could simplify creating release notes suitable for end users?
> > >
> > > I think this is a great suggestion. I would also like to see this happen.
> > >
> > > Having the maintainers or authors of the packages submit their bumps
> > > would ensure this does not become a burden for the central repository
> > > maintainer.
> > >
> > > An advantage of the maintainers of individual packages submitting
> > > their changes is they know the best timing for integrating their
> > > changes, and know what relevant details should be added to the bump
> > > commit message.
> >
> > I also would like to see this.  To be clear I mean the commit msg text above,
> > and not a ‘Impact’ token.
> >
> > The autobump script was deployed at a time in the project when _every_ repository
> > had the same maintainer, so it made some sense back then.  Today we have a much
> > more distributed set of maintainers.
> >
> > I’ve actually disabled the cron job that runs the script, because it needed
> > some rework to handle the new openbmc/openbmc repository layout.
> >
> > I plan on simply _not_ turning it back on, and let maintainers do their own
> > bumps like Joel has been doing for the kernel moving forward (Thanks Joel).
> > Unless there are major objections.
>
> I object!  At least 90% of the auto bumps today go through without
> issue.  This keeps our openbmc/openbmc master consistently up to date
> which I think is a big benefit to our community.  We're able to
> quickly find regressions with our automated tests which are constantly
> running against the new content coming in. I feel like we're adding
> more work with minimal benefit here.
>
> To summarize:
>
> - Adds a layer of manual work that has minimal benefits
> - Prevents us getting small continuous changes, and instead will
> encourage larger big chunks that are tougher to isolate fails on
> - Even if individual repo owners are forced to do their own bumps, I
> don't think many will address what started this thread, summaries of
> the changes and impacts of each commit.
>
> On the other side of thing I realize this is how yocto does it (manual
> bumps) and it does allow more individual control for repo maintainers.
> I just don't think at this point in our project, it's worth it.

Downstream we do manual bumps for our repo forks and it is a manual
process and we're looking at ways to automate it and provide the
information.  Right now, the bump, albeit manual ends up looking like:

"""
.... phosphor-pid-control: bump srcrev

Bump includes:
* 2524323 test: dbus: dbusactiveread
* 0ef1faf test: dbus: passive read interface
* 0df7c0f dbus: transition to interface for testing

Change-Id:
Signed-Off-By:
"""

So it's certainly not as informative as Joel's kernel ones in some
ways.  Our approach just lists, in order, the patches.  But like I
said, we're trying to automate that process to just include the
minimal details.

>
> Andrew
>
> >
> > thx - brad
> >
> > >
> > > Cheers,
> > >
> > > Joel

Re: Proposals for commit log policy

[Gunnar Mills]

On 8/28/2018 10:43 AM, Andrew Geissler wrote:

> On Tue, Aug 28, 2018 at 5:30 AM Brad Bishop <bradleyb@fuzziesquirrel.com> wrote:
>> I plan on simply _not_ turning it back on, and let maintainers do their own
>> bumps like Joel has been doing for the kernel moving forward (Thanks Joel).
>> Unless there are major objections.
> I object!  At least 90% of the auto bumps today go through without
> issue.  This keeps our openbmc/openbmc master consistently up to date
> which I think is a big benefit to our community.  We're able to
> quickly find regressions with our automated tests which are constantly
> running against the new content coming in. I feel like we're adding
> more work with minimal benefit here.

I object too. I don't think the benefit of manual bumps is worth the effort.

Thanks,
Gunnar

RE: Proposals for commit log policy

[Tanous, Ed]

> 
> I object too. I don't think the benefit of manual bumps is worth the effort.
> 

Another vote for "I don't think it's worth the effort".

Automatic bumps keep us up to date and make breakages happen and get fixed faster.  I would actually prefer that every gerrit review creates a manual bump to make sure the complete system builds with their given component change, but I realize this is hard to do.

-Ed

RE: Proposals for commit log policy

[Tanous, Ed]

Do you have an example of the version to version logs that are the result of the "3 full days contiguously of a qualified engineer" that you're asking the community to help you create?  I suspect if they were published (forgive me if they already are and I've just missed them) back to the project somewhere, and people found value, it might help the value proposition for developers a lot more.  Right now I'm seeing that is would be a lot of work on the part of code reviews, for only a minor gain.  This is coming from someone who has written release notes a lot.  I find the most value in release notes to be a consistent tone, language, and filtering, with a level of abstraction that is high enough to be useful.  I'd be surprised if we were able to get that kind of consistency from all the people on the project of varying backgrounds.

Another thing to consider is that there are a lot of non-native English speakers in the community, and while I would love it if everyone had a perfect grasp of the English language, I would _much_ rather that they know how to design, build, and test embedded firmware, while leaving the documentation to the people that excel at it.


> For bumps the task is "automatable" indeed. It's not actually as easy with
> Yocto as it may seem "to parse the git logs between the recipe bumps" (have
> you tried?), but it can be automated.

We don't dump the git logs, but my team does track the SHAs of the openbmc components in each revision we build for audit and reproducibility needs.  It's possible I'm missing something here, but it feels like dumping the logs would be a minor addition to the <20 line python script.  Our existing one just searches a glob of *.bb and *.bbappend files, greps for SHA1 changes, and fetches the relevant repos.  When I get a moment, I'll take a look and see if it can be easily generalized to dump logs for you if that helps some.

-Ed

Re: Proposals for commit log policy

[Kun Yi]

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]

My take is that we should keep the easy things easy. i.e. keep auto-bumping
most of the repos.

Would people be interested in something like an auto-bump configuration?
For example a optional configuration file that specifies the git log
format, or some option to 'turn auto-bump off for this repo'.

On Tue, Aug 28, 2018 at 3:21 PM Tanous, Ed <ed.tanous@intel.com> wrote:

> >
> > I object too. I don't think the benefit of manual bumps is worth the
> effort.
> >
>
> Another vote for "I don't think it's worth the effort".
>
> Automatic bumps keep us up to date and make breakages happen and get fixed
> faster.  I would actually prefer that every gerrit review creates a manual
> bump to make sure the complete system builds with their given component
> change, but I realize this is hard to do.
>
> -Ed
>


-- 
Regards,
Kun

[-- Attachment #2: Type: text/html, Size: 1296 bytes --]

Re: Proposals for commit log policy

[Brad Bishop]

> On Aug 28, 2018, at 6:20 PM, Tanous, Ed <ed.tanous@intel.com> wrote:
> 
>> 
>> I object too. I don't think the benefit of manual bumps is worth the effort.
>> 
> 
> Another vote for "I don't think it's worth the effort".
> 
> Automatic bumps keep us up to date and make breakages happen and get fixed faster.  I would actually prefer that every gerrit review creates a manual bump to make sure the complete system builds with their given component change, but I realize this is hard to do.
> 
> -Ed

The autobump script stopped working when we switched to yocto-style subtrees.

I fixed it up (by re-writing it in python…it was in Perl) and re-deployed, given
all the objections.

-brad

Re: Proposals for commit log policy

[Patrick Venture]

On Tue, Sep 4, 2018 at 1:20 PM Brad Bishop <bradleyb@fuzziesquirrel.com> wrote:
>
>
>
> > On Aug 28, 2018, at 6:20 PM, Tanous, Ed <ed.tanous@intel.com> wrote:
> >
> >>
> >> I object too. I don't think the benefit of manual bumps is worth the effort.
> >>
> >
> > Another vote for "I don't think it's worth the effort".
> >
> > Automatic bumps keep us up to date and make breakages happen and get fixed faster.  I would actually prefer that every gerrit review creates a manual bump to make sure the complete system builds with their given component change, but I realize this is hard to do.
> >
> > -Ed
>
> The autobump script stopped working when we switched to yocto-style subtrees.
>
> I fixed it up (by re-writing it in python…it was in Perl) and re-deployed, given
> all the objections.

Yay.  No more perl!

>
> -brad

Re: Proposals for commit log policy

[Alexander Amelkin]

[-- Attachment #1.1: Type: text/plain, Size: 510 bytes --]

05.09.2018 02:38, Patrick Venture wrote:
>
>> I fixed it up (by re-writing it in python…it was in Perl) and re-deployed, given
>> all the objections.
> Yay.  No more perl!
Argh! Why do you guys love this ugly python so much? Perl is so much more beautiful and comfortable and kludge-less (like has non-ugly regexps built in)... No semicolons? Indentation defining code block limits? Oh my... Nonetheless, ugly technologies tend to win, it seems.

No need to reply. Just my old-timer's muttering...


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]