[Starlingx-discuss] Python Based VBox Installer

[Penney, Don]

> -----Original Message-----
> From: Saul Wold [mailto:sgw at linux.intel.com]
> Sent: Thursday, February 21, 2019 6:19 PM
> To: starlingx-discuss at lists.starlingx.io
> Subject: Re: [Starlingx-discuss] Python Based VBox Installer
> 
> 
> 
> On 2/21/19 10:19 AM, Cordoba Malibran, Erich wrote:
> > On Thu, 2019-02-21 at 09:15 -0500, Curtis wrote:
> >>
> >>
> >> On Thu, Feb 21, 2019 at 8:46 AM Eslimi, Dariush <Dariush.Eslimi at windr
> >> iver.com> wrote:
> >>> Hi,
> >>>
> >>> I see Spec as a high level design for a code that going to be
> >>> developed, and to make sure it aligns with how community is going
> >>> to use it and to make everybody aware of what is going to do, so
> >>> others can chime in and say things that can address multiple needs
> >>> and influence its design. Let's call this apple.
> >>>
> >>> Now I see the case of a code that has been developed before even
> >>> the community existed, and now been presented to the community to
> >>> fill a gap and improve productivity, no time to change the design
> >>> and would be up to community to accept or reject it as is. A case
> >>> of donation really, I call this one Orange.
> >>>
> >>> For Orange I think Spec is not the right word, we need a
> >>> wiki/readme to see what it does, and how to use it. It is too late
> >>> to produce a spec to influence the design.
> >>>
> >>
> >> OpenStack Operators have a tools landing repository [1] that
> >> didn't/doesn't have the same level of requirements the rest of the
> >> OpenStack code typically does, so organizations could open source
> >> internal. potentially untested, scripts but not have to necessarily
> >> adhere to all of the usual requirements. Over time the code in that
> >> repository could be improved and moved out once it made sense.
> >>
> >> I think it's worthwhile to have useful internal code open sourced,
> >> and in some cases make it a bit easier to do so, and if the code/tool
> >> gets used then start improving it following general standards. This
> >> would require a new repository.
> >>
> >> Just a thought. :)
> >>
> >
> > I really like the idea of having a new repository as a landing zone
> > where things can be stabilize over time. When I started reviewing this
> > tool I noticed that my comments were more focused on design (features
> > that we might not need, restructure of cli arguments and so on) and I
> > understand that fixing this is outside of the scope of sharing an
> > internal tool.
> >
> > So, should be go in the path of creating a new repository? or can we
> > use an "experimental" folder within stx-tools?
> >
> > What do you think about this?
> >
> I think that this can land in the stx-tools directory along with the
> other deployment scripts. As was pointed out in the meeting this
> morning, this code has been in use within WindRiver for a while now and
> they are making it available to the community.
> 
> I think that the initial PR needed to have a better commit message
> explaining this and a little more thought put into the request before it
> got reviewed, such as removing the .pyc files, licensing, documentation,
> the first two where show stopper for me, but easily fixed, documentation
> could be added later.
> 
> Sau!
> 

Things like the inadvertent inclusion of pyc files (which were already noted in multiple comments in the review) and missing license identifiers seem to be minor and easily correctable mistakes, and maybe not completely unexpected for a “new contributor”. As well, I had also noted in a review comment that this was a long-existing tool that was being published, referencing the email from Numan and asking for the commit message to be updated to explain this.

As well, note that this was an optional productivity aid. It does not impact the build, it does not impact any software. It is a tool to help people launch StarlingX in a VirtualBox environment, to aid them in installing and configuring the system. Nobody is required to use it.

With that said, the review had been given two -2 votes from cores. As I understand it, this is not a minor thing. From the openstack guidelines, a -2 “is to indicate to the submitter that any further time they spend on the change will almost certainly be wasted.” Having two -2 votes on the update is very significant, thus the decision to abandon the review. The openstack guidelines describing -1 and -2 votes seems pretty clear here:
https://docs.openstack.org/project-team-guide/review-the-openstack-way.html#code-review-minus-2

Cheers,
Don.