<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">On Wed, Feb 27, 2019 at 9:57 AM Waheed, Numan <<a href="mailto:Numan.Waheed@windriver.com">Numan.Waheed@windriver.com</a>> wrote:<br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Review request for Python Bases VBox Installer has been restored. Kindly remove your -2 votes and add the comments that you would like to get fixed with -1. <br>
<br>
Some of the earlier comments have been already addressed. If there are new comments, we will address them accordingly.<br></blockquote><div><br></div><div><div style="font-size:small" class="gmail_default">Thanks! It'll be great to see this code get merged. :)</div><div style="font-size:small" class="gmail_default"><br></div><div style="font-size:small" class="gmail_default">Thanks,</div><div style="font-size:small" class="gmail_default">Curtis</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks,<br>
<br>
Numan<br>
<br>
-----Original Message-----<br>
From: Saul Wold <<a href="mailto:sgw@linux.intel.com" target="_blank">sgw@linux.intel.com</a>> <br>
Sent: February-25-19 2:51 PM<br>
To: <a href="mailto:starlingx-discuss@lists.starlingx.io" target="_blank">starlingx-discuss@lists.starlingx.io</a><br>
Subject: Re: [Starlingx-discuss] Python Based VBox Installer<br>
<br>
<br>
<br>
On 2/25/19 8:11 AM, Curtis wrote:<br>
> On Mon, Feb 25, 2019 at 10:51 AM Penney, Don <<a href="mailto:Don.Penney@windriver.com" target="_blank">Don.Penney@windriver.com</a> <br>
> <mailto:<a href="mailto:Don.Penney@windriver.com" target="_blank">Don.Penney@windriver.com</a>>> wrote:<br>
> <br>
> <br>
> <br>
>      > -----Original Message-----<br>
>      > From: Saul Wold [mailto:<a href="mailto:sgw@linux.intel.com" target="_blank">sgw@linux.intel.com</a><br>
>     <mailto:<a href="mailto:sgw@linux.intel.com" target="_blank">sgw@linux.intel.com</a>>]<br>
>      > Sent: Thursday, February 21, 2019 6:19 PM<br>
>      > To: <a href="mailto:starlingx-discuss@lists.starlingx.io" target="_blank">starlingx-discuss@lists.starlingx.io</a><br>
>     <mailto:<a href="mailto:starlingx-discuss@lists.starlingx.io" target="_blank">starlingx-discuss@lists.starlingx.io</a>><br>
>      > Subject: Re: [Starlingx-discuss] Python Based VBox Installer<br>
>      ><br>
>      ><br>
>      ><br>
>      > On 2/21/19 10:19 AM, Cordoba Malibran, Erich wrote:<br>
>      > > On Thu, 2019-02-21 at 09:15 -0500, Curtis wrote:<br>
>      > >><br>
>      > >><br>
>      > >> On Thu, Feb 21, 2019 at 8:46 AM Eslimi, Dariush<br>
>     <Dariush.Eslimi@windr<br>
>      > >> <a href="http://iver.com" rel="noreferrer" target="_blank">iver.com</a> <<a href="http://iver.com" rel="noreferrer" target="_blank">http://iver.com</a>>> wrote:<br>
>      > >>> Hi,<br>
>      > >>><br>
>      > >>> I see Spec as a high level design for a code that going to be<br>
>      > >>> developed, and to make sure it aligns with how community is going<br>
>      > >>> to use it and to make everybody aware of what is going to do, so<br>
>      > >>> others can chime in and say things that can address multiple<br>
>     needs<br>
>      > >>> and influence its design. Let's call this apple.<br>
>      > >>><br>
>      > >>> Now I see the case of a code that has been developed before even<br>
>      > >>> the community existed, and now been presented to the community to<br>
>      > >>> fill a gap and improve productivity, no time to change the design<br>
>      > >>> and would be up to community to accept or reject it as is. A case<br>
>      > >>> of donation really, I call this one Orange.<br>
>      > >>><br>
>      > >>> For Orange I think Spec is not the right word, we need a<br>
>      > >>> wiki/readme to see what it does, and how to use it. It is too<br>
>     late<br>
>      > >>> to produce a spec to influence the design.<br>
>      > >>><br>
>      > >><br>
>      > >> OpenStack Operators have a tools landing repository [1] that<br>
>      > >> didn't/doesn't have the same level of requirements the rest of the<br>
>      > >> OpenStack code typically does, so organizations could open source<br>
>      > >> internal. potentially untested, scripts but not have to<br>
>     necessarily<br>
>      > >> adhere to all of the usual requirements. Over time the code in<br>
>     that<br>
>      > >> repository could be improved and moved out once it made sense.<br>
>      > >><br>
>      > >> I think it's worthwhile to have useful internal code open sourced,<br>
>      > >> and in some cases make it a bit easier to do so, and if the<br>
>     code/tool<br>
>      > >> gets used then start improving it following general standards.<br>
>     This<br>
>      > >> would require a new repository.<br>
>      > >><br>
>      > >> Just a thought. :)<br>
>      > >><br>
>      > ><br>
>      > > I really like the idea of having a new repository as a landing zone<br>
>      > > where things can be stabilize over time. When I started<br>
>     reviewing this<br>
>      > > tool I noticed that my comments were more focused on design<br>
>     (features<br>
>      > > that we might not need, restructure of cli arguments and so on)<br>
>     and I<br>
>      > > understand that fixing this is outside of the scope of sharing an<br>
>      > > internal tool.<br>
>      > ><br>
>      > > So, should be go in the path of creating a new repository? or<br>
>     can we<br>
>      > > use an "experimental" folder within stx-tools?<br>
>      > ><br>
>      > > What do you think about this?<br>
>      > ><br>
>      > I think that this can land in the stx-tools directory along with the<br>
>      > other deployment scripts. As was pointed out in the meeting this<br>
>      > morning, this code has been in use within WindRiver for a while<br>
>     now and<br>
>      > they are making it available to the community.<br>
>      ><br>
>      > I think that the initial PR needed to have a better commit message<br>
>      > explaining this and a little more thought put into the request<br>
>     before it<br>
>      > got reviewed, such as removing the .pyc files, licensing,<br>
>     documentation,<br>
>      > the first two where show stopper for me, but easily fixed,<br>
>     documentation<br>
>      > could be added later.<br>
>      ><br>
>      > Sau!<br>
>      ><br>
> <br>
>     Things like the inadvertent inclusion of pyc files (which were<br>
>     already noted in multiple comments in the review) and missing<br>
>     license identifiers seem to be minor and easily correctable<br>
>     mistakes, and maybe not completely unexpected for a “new<br>
>     contributor”. As well, I had also noted in a review comment that<br>
>     this was a long-existing tool that was being published, referencing<br>
>     the email from Numan and asking for the commit message to be updated<br>
>     to explain this.<br>
> <br>
>     As well, note that this was an optional productivity aid. It does<br>
>     not impact the build, it does not impact any software. It is a tool<br>
>     to help people launch StarlingX in a VirtualBox environment, to aid<br>
>     them in installing and configuring the system. Nobody is required to<br>
>     use it.<br>
> <br>
>     With that said, the review had been given two -2 votes from cores.<br>
>     As I understand it, this is not a minor thing. From the openstack<br>
>     guidelines, a -2 “is to indicate to the submitter that any further<br>
>     time they spend on the change will almost certainly be wasted.”<br>
>     Having two -2 votes on the update is very significant, thus the<br>
>     decision to abandon the review. The openstack guidelines describing<br>
>     -1 and -2 votes seems pretty clear here:<br>
>     <br>
> <a href="https://docs.openstack.org/project-team-guide/review-the-openstack-way" rel="noreferrer" target="_blank">https://docs.openstack.org/project-team-guide/review-the-openstack-way</a><br>
> .html#code-review-minus-2<br>
> <br>
> <br>
> Sounds like we need to come to a consensus as to when a -2 is <br>
> appropriate. I personally don't think a -2 is appropriate for a first <br>
> contribution, regardless of circumstances. Obviously we can't commit <br>
> .pyc files and need licensing, but I definitely would prefer not to <br>
> see -2s in this situation.<br>
> <br>
As I mentioned above, I was probably over zealous on marking it -2, consider it withdrawn.<br>
<br>
> I think this code is important and we need to get this back on track. <br>
> This is a good "teachable moment" maybe for all sides? :)<br>
><br>
Lesson learned!<br>
<br>
Sau!<br>
<br>
<br>
> Thanks,<br>
> Curtis<br>
> <br>
>     Cheers,<br>
>     Don.<br>
> <br>
>     _______________________________________________<br>
>     Starlingx-discuss mailing list<br>
>     <a href="mailto:Starlingx-discuss@lists.starlingx.io" target="_blank">Starlingx-discuss@lists.starlingx.io</a><br>
>     <mailto:<a href="mailto:Starlingx-discuss@lists.starlingx.io" target="_blank">Starlingx-discuss@lists.starlingx.io</a>><br>
>     <br>
> <a href="http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss" rel="noreferrer" target="_blank">http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss</a><br>
> <br>
> <br>
> <br>
> --<br>
> Blog: <a href="http://serverascode.com" rel="noreferrer" target="_blank">serverascode.com</a> <<a href="http://serverascode.com" rel="noreferrer" target="_blank">http://serverascode.com</a>><br>
> <br>
> _______________________________________________<br>
> Starlingx-discuss mailing list<br>
> <a href="mailto:Starlingx-discuss@lists.starlingx.io" target="_blank">Starlingx-discuss@lists.starlingx.io</a><br>
> <a href="http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss" rel="noreferrer" target="_blank">http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss</a><br>
> <br>
<br>
_______________________________________________<br>
Starlingx-discuss mailing list<br>
<a href="mailto:Starlingx-discuss@lists.starlingx.io" target="_blank">Starlingx-discuss@lists.starlingx.io</a><br>
<a href="http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss" rel="noreferrer" target="_blank">http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss</a><br>
_______________________________________________<br>
Starlingx-discuss mailing list<br>
<a href="mailto:Starlingx-discuss@lists.starlingx.io" target="_blank">Starlingx-discuss@lists.starlingx.io</a><br>
<a href="http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss" rel="noreferrer" target="_blank">http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss</a><br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature">Blog: <a href="http://serverascode.com" target="_blank">serverascode.com</a></div></div>