[Starlingx-discuss] Python Based VBox Installer

Cordoba Malibran, Erich erich.cordoba.malibran at intel.com
Mon Feb 25 16:51:21 UTC 2019


On Mon, 2019-02-25 at 11:11 -0500, Curtis wrote:
> On Mon, Feb 25, 2019 at 10:51 AM Penney, Don <Don.Penney at windriver.co
> m> wrote:
> > 
> > > -----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.Eslim
> > i 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
> > 
> 
> Sounds like we need to come to a consensus as to when a -2 is
> appropriate. I personally don't think a -2 is appropriate for a first
> contribution, regardless of circumstances. Obviously we can't commit
> .pyc files and need licensing, but I definitely would prefer not to
> see -2s in this situation.
> 
> I think this code is important and we need to get this back on track.
> This is a good "teachable moment" maybe for all sides? :)
> 

I agree, we need to get it back. I gave my -2 in the understanding that
every new functionality to the project needs a spec and an approval
process first. Now that this has been discussed and it's clear we can
have this kind of contributions, then I can remove the -2 (the review
needs to be restored first) and continue with the review.

-Erich


> Thanks,
> Curtis
> 
>  
> > Cheers,
> > Don.
> > 
> > _______________________________________________
> > Starlingx-discuss mailing list
> > Starlingx-discuss at lists.starlingx.io
> > http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discus
> > s
> 
> 
> _______________________________________________
> Starlingx-discuss mailing list
> Starlingx-discuss at lists.starlingx.io
> http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss


More information about the Starlingx-discuss mailing list