Choose a standard for menuitem capitalization, and apply it to the per-push Action menu

RESOLVED FIXED

Status

Tree Management
Treeherder
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: philor, Assigned: torroid)

Tracking

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
I think I've finally milked all the comedy I can out of the menuitem "Add new Jobs" looking like it should add more Steve Jobs to a push.

To the extent we have a standard for menuitems, it's probably Sentence case rather than Title Case (and Certainly isn't random Case), and instead of 

Add new Jobs
Mark with Bugherder
BuildAPI
Trigger Missing jobs
Trigger All Talos jobs
Set as top of range
Set as bottom of range

we should have

Add new jobs
Mark with Bugherder
BuildAPI
Trigger missing jobs
Trigger all Talos jobs
Set as top of range
Set as bottom of range

though it's a little ambiguous whether Talos is really a proper noun or something we've genericized into a synonym for perf, and we would still call a perf job that didn't use the Talos harness a talos job.

I was tempted to argue for Title Case instead, since the ambiguity of what case is being used in "Mark with Bugherder" might explain why the pulse_actions items were added with such random case, but it turns out that the Apple HIG's version of Title Case calls for lowercase prepositions, so I would have to argue for GNOME's Header Case to get its rule of "capitalize all words of four or more letters," and I'm not willing to go there, particularly not considering what that would do to the top/bottom of range items.
Google's material design guidelines for writing seem pretty thoughtful: https://material.google.com/style/writing.html
Maybe we should just use those? Cam, is this something we could get Buddy group's advice on?

In the mean time, I think we should just do what :philor suggests. Glenn, do you want to give that a try? You should be able to find the strings that need to be changed with "git grep". You can test your changes with a local copy of treeherder: http://treeherder.readthedocs.io/ui/index.html
Assignee: nobody → glenn28f
Flags: needinfo?(cdawson)
Sure, yeah.  I pinged them in an email and asked if they'd chime in here.  Or I'll chime in for them.  I mean, the right answer is USUALLY just whatever Philor says anyway.  But another opinion won't hurt...  :)
Flags: needinfo?(cdawson)
(Assignee)

Comment 3

a year ago
Hi! I'd like to give it a try.
Saw this update in passing, so I'll mark the bug as assigned also.
Status: NEW → ASSIGNED

Comment 5

a year ago
Created attachment 8796472 [details] [review]
[treeherder] glenn124f:fix_capitalization > mozilla:master
(Assignee)

Updated

a year ago
Attachment #8796472 - Flags: review?(wlachance)
Comment on attachment 8796472 [details] [review]
[treeherder] glenn124f:fix_capitalization > mozilla:master

This looks fine, however it looks like there are some merge conflicts. Could you pull the latest master and rebase your branch against it?

Also, the name of the commit should refer to what's changed, not be the bug title. In this case something like `Bug 1301986 - Standardize action menu capitalization` would be good.
Attachment #8796472 - Flags: review?(wlachance)
(Assignee)

Comment 7

a year ago
Im not able to figure out about the conflicts. I tried pulling and rebasing. Please could you help me with this.
(In reply to Glenn Fernandes [:torroid] from comment #7)
> Im not able to figure out about the conflicts. I tried pulling and rebasing.
> Please could you help me with this.

It can be tricky. There are lots of guides available on the web which can help understanding how to resolve conflicts. Failing that, drop by #treeherder and paste a log of what you've tried. Someone should be able to help.

Comment 9

a year ago
Created attachment 8797038 [details] [review]
[treeherder] glenn124f:fix_capitalization > mozilla:master
(Assignee)

Comment 10

a year ago
Comment on attachment 8797038 [details] [review]
[treeherder] glenn124f:fix_capitalization > mozilla:master

Removed the conflicts. I hope this is fine.
Attachment #8797038 - Flags: review?(cdawson)
Hi Glenn, fyi we normally push to the same PR for changes, rather than closing a PR and re-opening a new PR. It makes the GitHub PR closed-list more clear and searchable, rather than containing a bunch of empty entries.
(Assignee)

Comment 12

a year ago
I'm really sorry about that. I had some conflicts and tried to avoid deleting commits, I accidentally deleted the commits while doing a hard reset. I'll take care to avoid this in the future.
Ya, in that scenario you'd force push to the original PR with your changes post reset, and re-use the PR. This is a simple change, so it's not a big deal here to have wiped your history. In more typical/complex bugs you want to avoid deleting commits after review begins; but you generally always want to use one PR.
Comment on attachment 8797038 [details] [review]
[treeherder] glenn124f:fix_capitalization > mozilla:master

This looks good to me.  Thanks for the fixes!
Attachment #8797038 - Flags: review?(cdawson) → review+

Comment 15

a year ago
Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/86a3f974f10ed7ac0cc49b41d06ace4afaf3a70b
Bug 1301986 - Standardize action menu_capitalization (#1889)
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.