Code review comment for lp:~jelmer/launchpad/bug471148-ui

Revision history for this message
Michael Nelson (michael.nelson) wrote :

So just an update of the label text for now (other tiny improvements possible and outlined below, but can be made later).

16:43 < jelmer> noodles: Could you perhaps do a quick UI review?
16:44 < noodles> yeah, I'm just trying to get the 7th branch in my pipe through too.
16:44 < jelmer> We've just added a checkbox
16:44 < noodles> Sure.
16:44 < jelmer> http://code.launchpad.net/~jelmer/launchpad/bug471148-ui is the code
16:44 < edbot> Bug 471148 is private
16:44 < jelmer> I'm uploading a screenshot
16:44 < noodles> Great.
16:45 < noodles> Have you got an MP ready? (You can make it a "work in progress" one).
16:46 < jelmer> https://code.edge.launchpad.net/~jelmer/launchpad/bug471148-ui/+merge/20208 is the mp
16:46 < edbot> Bug 471148 is private
16:46 < jelmer> abel has already reviewed the code
16:50 < noodles> jelmer: you guys are aware of the conflict right?
16:51 < noodles> oh, it's not in the diff...
16:51 < noodles> ah, yes it is :)
16:52 < noodles> Just a conflict with bigjools' configure changes from yesterday.
16:52 < jelmer> ah
16:52 < jelmer> I'll have a look
16:52 < jelmer> thanks
16:53 < bigjools> noodles: where are you seeing a conflict notified?
16:53 < noodles> bigjools: on jelmer's MP
16:53 < bigjools> oh - lol :)
16:53 < bigjools> thought it was db_lp
16:58 < jelmer> noodles: Muharem is uploading the screenshot, my launchpad instance is b0rked for some reason
16:59 < noodles> jelmer: I've merged it locally, so I'll run it here.
16:59 < noodles> But the screenshot will still be handy.
17:00 < al-maisan> noodles: here you go: https://devpad.canonical.com/~muharem/bug471148-2.png
17:00 < edbot> Bug 471148 is private
17:00 < noodles> Ta
17:03 < bigjools> fixed, thanks :)
17:04 < noodles> al-maisan, jelmer, wording-wise, I think "Allow ARM builds for this archive" (note: without a period) would be more consistent?
17:05 < al-maisan> hm .. hm .. OK
17:06 < noodles> If you've got other thoughts, just shout them out, I'm just comparing to the other checkboxes on that form (and looking for other examples).
17:07 * jelmer fix0rs
17:07 < noodles> And oh, would you mind cleaning up the capitalization on that form, there's lots of Headline Case for all the labels, that should be
                 sentence case (https://dev.launchpad.net/UserInterfaceWording)
17:07 < noodles> eg
17:07 < noodles> Require Virtualized Builder, Buildd Secret etc.
17:08 < noodles> (they probably have never been updated since we had the UIwording guidelines).
17:09 < noodles> But I'll understand if you don't want to risk test breakages etc. this late on Fri. afternoon.
17:09 < bigjools> hehe jelmer changed his gravatar again :)
17:09 < jelmer> bigjools: I just uploaded a bunch of images and gravatar makes it very easy to switch :-)
17:09 < bigjools> you need a floaty head!
17:09 < jelmer> noodles: Yeah, it'd be nice if we could just land this for now
17:09 < noodles> so ui=me just with the updated label for your checkbox.
17:09 < noodles> Yep.
17:10 < jelmer> noodles: I've made the wording change on the checkbox label
17:10 < noodles> Great.
17:10 < jelmer> noodles: Thanks!
17:10 < noodles> NP.

review: Approve (ui)

« Back to merge proposal