← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~ilasc/launchpad:oci-recipe-push-rules-edit into launchpad:master

 

I'm adding a few comments about the visual of the edit pages. Since they might (or might not) change the code part, I'll review the code after you take a look on these points (let me know if something doesn't make sense or you don't agree):

- When I enter the "Edit OCI push rules for <recipe>" page, the password fields to add a new credential are not password fields (I can see what I type).

- Shouldn't the "Use existing credentials" and "Add new credentials" be radio buttons? Is it expected to be able to both add and use an existing credential at the same time?

- After adding a push rule, it might be interesting to have a separation between the table of existing rules and the form below to add a new one. Maybe an <h2> tag with "Add a new push rule" before the form would do.

- On the edit push rule table, since we are repeating the field names on each row, maybe we can drop the table header, the same way we have the +edit-credentials table.

- On the edit push rule table, do we really need a link to edit the credentials on each row, given that every link points to the same +edit-credentials page? Maybe we should put this link only once in the page.
-- 
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/386371
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:oci-recipe-push-rules-edit into launchpad:master.


References