[Starlingx-discuss] Please Review: Requirements for commit messages

Fri Sep 13 17:33:22 UTC 2019

A good point, Eric.  As a reviewer, I think it would be fair to say "I don't understand this change, can you please elaborate in the commit message or in a Story?".  But if the change is well understood by the reviewers (because it has a clear commit message), why ask the developer to do more work than needed?

         brucej

-----Original Message-----
From: MacDonald, Eric [mailto:Eric.MacDonald at windriver.com] 
Sent: Friday, September 13, 2019 10:28 AM
To: Jones, Bruce E <bruce.e.jones at intel.com>; Eslimi, Dariush <Dariush.Eslimi at windriver.com>; Khalil, Ghada <Ghada.Khalil at windriver.com>; Saul Wold <sgw at linux.intel.com>; starlingx-discuss at lists.starlingx.io
Subject: RE: [Starlingx-discuss] Please Review: Requirements for commit messages

There seem to be a lot of small changes that are being posted without quality or traceable reasoning and I feel that creating a SB and tasks for like changes helps reviewers and project/content managers.

> -----Original Message-----
> From: Jones, Bruce E [mailto:bruce.e.jones at intel.com]
> Sent: Friday, September 13, 2019 12:54 PM
> To: Eslimi, Dariush; MacDonald, Eric; Khalil, Ghada; Saul Wold; 
> starlingx-discuss at lists.starlingx.io
> Subject: RE: [Starlingx-discuss] Please Review: Requirements for 
> commit messages
> Importance: High
> 
> It's just an example, Dariush, of an internal change that would not 
> impact an end user.  Insert your own example if needed. :)
> 
> Commit messages can provide plenty of context.  If they are written 
> well, of course, which the reviewers can verify / enforce.
> 
> I'm not opposed to using SB and LP to document changes.  But I don't 
> think it's necessary to mandate it for every single change.
> 
>      brucej
> 
> 
> -----Original Message-----
> From: Eslimi, Dariush [mailto:Dariush.Eslimi at windriver.com]
> Sent: Friday, September 13, 2019 8:53 AM
> To: Jones, Bruce E <bruce.e.jones at intel.com>; MacDonald, Eric 
> <Eric.MacDonald at windriver.com>; Khalil, Ghada 
> <Ghada.Khalil at windriver.com>; Saul Wold <sgw at linux.intel.com>; 
> starlingx- discuss at lists.starlingx.io
> Subject: RE: [Starlingx-discuss] Please Review: Requirements for 
> commit messages
> 
> I would argue that is very bad example, why would a single project all 
> of sudden wants to switch to Python3?
> Is this part of release goal to start using Python 3? Is this priority for the release?
> 
> Small change need more context so code reviewers know why such a change is proposed.
> 
> Dariush
> 
> -----Original Message-----
> From: Jones, Bruce E [mailto:bruce.e.jones at intel.com]
> Sent: September-13-19 11:37 AM
> To: MacDonald, Eric <Eric.MacDonald at windriver.com>; Khalil, Ghada 
> <Ghada.Khalil at windriver.com>; Saul Wold <sgw at linux.intel.com>; 
> starlingx-discuss at lists.starlingx.io
> Subject: Re: [Starlingx-discuss] Please Review: Requirements for 
> commit messages
> 
> -1.  Commit messages are update reasons and just as traceable.
> 
> Do we really need a SB entry for changes like "replace #!/usr/bin/python with #!/usr/bin/python3"?
> 
>          bruej
> 
> -----Original Message-----
> From: MacDonald, Eric [mailto:Eric.MacDonald at windriver.com]
> Sent: Friday, September 13, 2019 8:30 AM
> To: Khalil, Ghada <Ghada.Khalil at windriver.com>; Saul Wold 
> <sgw at linux.intel.com>; starlingx- discuss at lists.starlingx.io
> Subject: Re: [Starlingx-discuss] Please Review: Requirements for 
> commit messages
> 
> +2 Ghada's point. I'd like to avoid the slippery slope no update reason or traceability can lead to.
> 
> > -----Original Message-----
> > From: Khalil, Ghada [mailto:Ghada.Khalil at windriver.com]
> > Sent: Friday, September 13, 2019 11:11 AM
> > To: Saul Wold; starlingx-discuss at lists.starlingx.io
> > Subject: Re: [Starlingx-discuss] Please Review: Requirements for 
> > commit messages
> >
> > Hi Saul,
> > This was discussed previously and, as a community, we agreed that 
> > this is the guideline for starlingx submissions
> >
> > From
> > https://wiki.openstack.org/wiki/StarlingX/CodeSubmissionGuidelines
> > Link your review to a StoryBoard Story or Launchpad Bug For 
> > traceability, always link your code change to a story or bug. The 
> > story/bug will give reviewers context for the code changes. This 
> > will also be used to help determine the relative priority of the code changes.
> >
> >
> > I personally don't see a big barrier to creating a generic 
> > storyboard for spelling or cleanup tasks. Happy to hear from others their opinion.
> > I'm not terribly concerned about a few small updates without a link, 
> > but we should all aim to favor traceability (and document the purpose for the changes we're making).
> >
> > Regards,
> > Ghada
> >
> > -----Original Message-----
> > From: Saul Wold [mailto:sgw at linux.intel.com]
> > Sent: Thursday, September 12, 2019 5:53 PM
> > To: starlingx-discuss at lists.starlingx.io
> > Subject: [Starlingx-discuss] Please Review: Requirements for commit 
> > messages
> >
> >
> > StarlingX Folks:
> >
> > I would like to clarify the requirement or lack there of for having Bugs or Stories in commit messages.
> > There are patches that are clearly addressing bug fixes and those 
> > should have a Closes-Bug; there are commits that are clearly 
> > implementing features that should have
> the Story/Task tags.
> >
> > There are also commits that fall in between, they are not directly 
> > related to a given feature or have a bug filed, they are typically 
> > smaller and are fairly self-documenting or have good commit messages 
> > explaining why the change is
> needed, not just what the change does.
> >
> > Ultimately Closes-Bugs and Story tags will help reviewers and other 
> > get the infomration needed to create release notes, but we don't 
> > need to document every change like spelling, indentation, fixing compiler warnings in Release notes.
> >
> > I believe that we should have some flexibility in allowing for 
> > changes without related launchpad or story connections. This can 
> > help reduce the barrier to entry for small changes that community members may want to propose.
> >
> > Thoughts?
> >
> > Sau!
> >
> > _______________________________________________
> > Starlingx-discuss mailing list
> > Starlingx-discuss at lists.starlingx.io
> > http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
> > _______________________________________________
> > Starlingx-discuss mailing list
> > Starlingx-discuss at lists.starlingx.io
> > http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
> _______________________________________________
> Starlingx-discuss mailing list
> Starlingx-discuss at lists.starlingx.io
> http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
> _______________________________________________
> Starlingx-discuss mailing list
> Starlingx-discuss at lists.starlingx.io
> http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss