launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24906
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