[Toaster] V2: [review-request] configure variables page: dreyna/hardcoded_submit_7246_7371

[Damian, Alexandru]

taken for submission, with whitespace changes.

thanks,
Alex

On Tue, Mar 3, 2015 at 9:46 AM, Barros Pena, Belen <
belen.barros.pena at intel.com> wrote:

>
>
> On 02/03/2015 20:23, "Reyna, David" <david.reyna at windriver.com> wrote:
>
> >Hi Belén,
> >
> >> if you delete all our preset variables and then
> >> add back IMAGE_INSTALL_append the tooltip of
> >> the delete icon doesn't come up.
> >
> >Fixed and pushed. Code was in the wrong {% if %}.
>
> Thanks!
>
> >
> >> The other one is more of a problem: the validation against variables
> >> currently set in the page has stopped working. I was able to add 2
> >> distros
> >> :/
> >
> >I cannot replicate this, adding a second "DISTRO" (or any other
> >variable). How did you do this?
> >
> >What I could replicate was for example adding "distro" in addition to
> >"DISTRO". This upper/lower case test is now fixed and pushed, in addition
> >to dangling error class markers when there were cumulative errors.
>
> agreed: I am not sure what happened yesterday when I tried, but whatever
> it was, it's working properly now :)
>
> So from the UI side this is ready to submit upstream.
>
> Thanks!
>
> Belén
>
>
>
> >
> >- David
> >
> >
> >> -----Original Message-----
> >> From: Barros Pena, Belen [mailto:belen.barros.pena at intel.com]
> >> Sent: Monday, March 02, 2015 3:37 AM
> >> To: Reyna, David
> >> Cc: toaster at yoctoproject.org
> >> Subject: Re: V2: [Toaster] [review-request] configure variables page:
> >> dreyna/hardcoded_submit_7246_7371
> >>
> >> Hi David,
> >>
> >> All the issues in my previous email are fixed :) The bad news is that
> >> I've
> >> found a couple more:
> >>
> >> One is pretty small: if you delete all our preset variables and then
> >> add
> >> back IMAGE_INSTALL_append using the 'add variable' form, the tooltip of
> >> the delete icon doesn't come up.
> >>
> >> The other one is more of a problem: the validation against variables
> >> currently set in the page has stopped working. I was able to add 2
> >> distros
> >> :/
> >>
> >> Thanks!
> >>
> >> Belén
> >>
> >> On 28/02/2015 04:45, "Reyna, David" <david.reyna at windriver.com> wrote:
> >>
> >> >Hi Belén.
> >> >
> >> >Ok, I have fixed the issues and re-pushed my commit
> >> >"dreyna/hardcoded_submit_7246_7371".
> >> >
> >> >[x] "If you add manually one of our default variables, I can briefly
> >> see
> >> >an
> >> >error alert coming up, then disappearing very quickly."
> >> >
> >> >This was a race condition between the 'postEditAjaxRequest' and the
> >> page
> >> >reload. Fixed.
> >> >
> >> >[x] "When you add back SDKMACHINE, when you click the 'change' icon
> >> the
> >> >Cancel button is disabled"
> >> >[x] "When you add back PACKAGE_CLASESS, the 'Save' button is disabled,
> >> so
> >> >you
> >> >cannot make any changes to the variable value"
> >> >
> >> >This was really odd and took time to figure out. It turns out that
> >> with a
> >> >soft reload these two buttons (and only these two buttons) get a
> >> >disabled="" attribute inserted magically. The fix was to do a hard
> >> reload
> >> >instead (which is anyway better in the long run).
> >> >
> >> >[x] "... I found the message a bit confusing ..."
> >> >
> >> >I went with the generic: "You cannot set this variable here. Please
> >> set
> >> >it in the project main page", with a link to that project's main page.
> >> >
> >> >[x] "The only issue I've found is that the MACHINE variable is showing
> >> up
> >> >..."
> >> >
> >> >Filter added for both the blacklist and {MACHINE,BBLAYERS} in the
> >> initial
> >> >page serve and the XHR serve.
> >> >
> >> >- David
> >> >
> >> >> -----Original Message-----
> >> >> From: Barros Pena, Belen [mailto:belen.barros.pena at intel.com]
> >> >> Sent: Friday, February 27, 2015 5:15 AM
> >> >> To: Reyna, David
> >> >> Cc: toaster at yoctoproject.org; DAMIAN, ALEXANDRU
> >> >> Subject: Re: [Toaster] [review-request] configure variables page:
> >> >> dreyna/hardcoded_submit_7246_7371
> >> >>
> >> >> I just found a couple of other things, sorry:
> >> >>
> >> >> * If you add manually one of our default variables, I can briefly
> >> see
> >> >> an
> >> >> error alert coming up, then disappearing very quickly.
> >> >>
> >> >> * When you add back SDKMACHINE, when you click the 'change' icon the
> >> >> Cancel button is disabled.
> >> >>
> >> >> * When you add back PACKAGE_CLASESS, the 'Save' button is disabled,
> >> so
> >> >> you
> >> >> cannot make any changes to the variable value
> >> >>
> >> >> Cheers
> >> >>
> >> >> Belén
> >> >>
> >> >>
> >> >>
> >> >> On 27/02/2015 12:10, "Barros Pena, Belen"
> >> <belen.barros.pena at intel.com>
> >> >> wrote:
> >> >>
> >> >> >Hi David,
> >> >> >
> >> >> >This is working great. I only have a couple of small comments
> >> >> (inline):
> >> >> >
> >> >> >On 27/02/2015 04:08, "Reyna, David" <david.reyna at windriver.com>
> >> wrote:
> >> >> >
> >> >> >>Hi Belén,
> >> >> >>
> >> >> >>This commit ³dreyna/hardcoded_submit_7246_7371³ fixes these items:
> >> >> >>
> >> >> >>  7246 ³Hardcoded variables in /configuration page²
> >> >> >
> >> >> >The only issue I've found is that the MACHINE variable is showing
> >> up
> >> >> in
> >> >> >the page. I think we should filter it out, particularly because the
> >> >> >validation you have added to the 'add variable' form for preventing
> >> >> people
> >> >> >setting the machine in this page.
> >> >> >
> >> >> >>  7371 ³Toaster managed mode: configuration variables page allows
> >> >> empty
> >> >> >>inputs into database²
> >> >> >>
> >> >> >>
> >> >> >>  * Finally, I added/fixed code to enforced the blocking of the
> >> >> variables
> >> >> >>that are managed outside this page, specifically BBLAYERS and
> >> MACHINE
> >> >> >>which are handled by the project detail page.
> >> >> >
> >> >> >This is very nice, I found the message a bit confusing, because it
> >> >> gives
> >> >> >me a reason but does not tell me what to do. I know this might
> >> require
> >> >> >some hard coding, but could we have different messages for the
> >> MACHINE
> >> >> and
> >> >> >the BBLAYERS cases?
> >> >> >
> >> >> >So if you try to set the MACHINE, the message should say:
> >> >> >
> >> >> >"You cannot set this variable here. Please set the MACHINE in the
> >> >> project
> >> >> >main page"
> >> >> >
> >> >> >If you try to set BBLAYERS, the message should say:
> >> >> >
> >> >> >"You cannot set this variable here. Please add and remove layers in
> >> >> the
> >> >> >project main page"
> >> >> >
> >> >> >If this is too much and you rather use a single message, it could
> >> say:
> >> >> >
> >> >> >"You cannot set this variable here. Please set it in the project
> >> main
> >> >> >page"
> >> >> >
> >> >> >In all the above messages, "project main page" should be a link to
> >> the
> >> >> >project page.
> >> >> >
> >> >> >
> >> >> >I hope I am not asking too much.
> >> >> >
> >> >> >Thanks!!
> >> >> >
> >> >> >Belén
> >> >> >
> >> >> >>
> >> >> >>- David
> >> >> >>
> >> >> >>
> >> >> >
> >> >> >--
> >> >> >_______________________________________________
> >> >> >toaster mailing list
> >> >> >toaster at yoctoproject.org
> >> >> >https://lists.yoctoproject.org/listinfo/toaster
> >> >>
> >> >
> >>
> >
>
> --
> _______________________________________________
> toaster mailing list
> toaster at yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>



-- 
Alex Damian
Yocto Project
SSG / OTC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150309/fbfda4ab/attachment.html>