Re: [PATCH 0/4] Add delete layer and fix layer rev saving

[Michael Wood <michael.g.wood@intel.com>]

On 06/07/16 15:02, Smith, Elliot wrote:
> On 6 July 2016 at 14:33, Michael Wood <michael.g.wood@intel.com 
> <mailto:michael.g.wood@intel.com>> wrote:
>
>     On 06/07/16 14:28, Barros Pena, Belen wrote:
>
>
>         On 06/07/2016 14:22, "toaster-bounces@yoctoproject.org
>         <mailto:toaster-bounces@yoctoproject.org> on behalf of Barros
>         Pena, Belen" <toaster-bounces@yoctoproject.org
>         <mailto:toaster-bounces@yoctoproject.org> on behalf of
>         belen.barros.pena@intel.com
>         <mailto:belen.barros.pena@intel.com>> wrote:
>
>
>             On 04/07/2016 21:56, "toaster-bounces@yoctoproject.org
>             <mailto:toaster-bounces@yoctoproject.org> on behalf of
>             Michael Wood" <toaster-bounces@yoctoproject.org
>             <mailto:toaster-bounces@yoctoproject.org> on behalf of
>             michael.g.wood@intel.com
>             <mailto:michael.g.wood@intel.com>> wrote:
>
>                 Michael Wood (4):
>                   toaster: layerdetails api Fix saving of git revision
>                 of a layer
>                   toaster: api Add util function for returning the
>                 error response
>                   toaster: add Layer delete front end feature to
>                 layerdetails
>                   toaster: tests Add selenium test for layerdetails page
>
>
>
> A couple of comments so far:
>
> 1. I think I'm right in saying that "deleting" a layer permanently 
> removes it from the database. I think the confirmation dialog should 
> make it clear that this is what you're doing ("This layer won't be 
> available to other projects" or similar), as it's not obvious that 
> remove and delete are different operations. (I didn't get the 
> distinction to start with.)

The imported layers are project specific so it is deleted from the 
project, and also deleted from being added to your project if 
applicable. I'll leave this in Belen's corner though.


>
> 2. I see the "delete" link for all layers in the layer detail page, 
> not just for layers which I've imported. These layers can't be 
> deleted, so the dialog box doesn't change when you click "OK", as an 
> error is returned by the API.
>

Ah thanks good catch, will roll a v2.

> 3. The inline editing of layer dependencies doesn't quite feel right 
> to me: it's not 100% clear you're going to be adding layer 
> dependencies by using that box, you could just be adding a new layer. 
> I'd change the placeholder text to something like "Type the name of a 
> layer dependency to add" (ideally, something less clunky than that), 
> or perhaps change the button text to "Add layer dependency".
>

As this is the existing behaviour (for many releases) you would have to 
open a bug about this as it's out of the scope of this patch set.

Thanks

Michael

> Elliot
>
>             The fix to the revision changes for imported layers (8952)
>             works like a
>             charm: I can edit all the layer details now. I can also
>             delete an imported
>             layer \0/, but when I get redirected to the project
>             configuration page
>             after clicking the 'delete' button no notification
>             appears, so I am left
>             wondering if the layer was deleted or not.
>
>         Ignore me: browser cache playing tricks. But the question mark
>         really is
>         missing ;)
>
>
>     OK, I fixed the missing question mark in the branch, so if who
>     ever is reviewing pulls from the branch then it should be there.
>
>
>
>             I also spotted that the question in the delete
>             confirmation dialog is
>             missing the question mark.
>
>             Thanks!!
>
>             Belén
>
>             PS: I hate delete confirmation dialogs ... For the record ;)
>
>                 .../tests/browser/test_layerdetails_page.py       | 190
>                 +++++++++++++++++++++
>                 bitbake/lib/toaster/toastergui/api.py   | 121
>                 +++++++++++--
>                 .../toaster/toastergui/static/js/layerdetails.js  | 
>                 20 +++
>                 .../toaster/toastergui/static/js/projectpage.js   |  18 ++
>                 .../toaster/toastergui/templates/layerdetails.html | 
>                 18 +-
>                 bitbake/lib/toaster/toastergui/urls.py    |   5 +-
>                 bitbake/lib/toaster/toastergui/views.py   |  43 -----
>                 7 files changed, 360 insertions(+), 55 deletions(-)
>                 create mode 100644
>                 bitbake/lib/toaster/tests/browser/test_layerdetails_page.py
>
>                 -- 
>                 2.7.4
>
>                 -- 
>                 _______________________________________________
>                 toaster mailing list
>                 toaster@yoctoproject.org <mailto:toaster@yoctoproject.org>
>                 https://lists.yoctoproject.org/listinfo/toaster
>
>             -- 
>             _______________________________________________
>             toaster mailing list
>             toaster@yoctoproject.org <mailto:toaster@yoctoproject.org>
>             https://lists.yoctoproject.org/listinfo/toaster
>
>
>     -- 
>     _______________________________________________
>     toaster mailing list
>     toaster@yoctoproject.org <mailto:toaster@yoctoproject.org>
>     https://lists.yoctoproject.org/listinfo/toaster
>
>
>
>
> -- 
> Elliot Smith
> Software Engineer
> Intel Open Source Technology Centre
>
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ
> VAT No: 860 2173 47
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>