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

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Phil Ringnalda (:philor)]

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.

Comment 1

[William Lachance (:wlach)]

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

Comment 2

[Cameron Dawson [:camd]]

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...  :)

Comment 3

[Glenn Fernandes [:torroid]]

Hi! I'd like to give it a try.

Comment 4

[Jonathan French (:jfrench)]

Saw this update in passing, so I'll mark the bug as assigned also.

Comment 5

[GitHub Autolander Bot]

Attachment: [treeherder] glenn124f:fix_capitalization > mozilla:master

Comment 6

[William Lachance (:wlach)]

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.

Comment 7

[Glenn Fernandes [:torroid]]

Im not able to figure out about the conflicts. I tried pulling and rebasing. Please could you help me with this.

Comment 8

[William Lachance (:wlach)]

(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

[GitHub Autolander Bot]

Attachment: [treeherder] glenn124f:fix_capitalization > mozilla:master

Comment 10

[Glenn Fernandes [:torroid]]

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

Removed the conflicts. I hope this is fine.

Comment 11

[Jonathan French (:jfrench)]

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.

Comment 12

[Glenn Fernandes [:torroid]]

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.

Comment 13

[Jonathan French (:jfrench)]

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 14

[Cameron Dawson [:camd]]

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

This looks good to me.  Thanks for the fixes!

Comment 15

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/86a3f974f10ed7ac0cc49b41d06ace4afaf3a70b
Bug 1301986 - Standardize action menu_capitalization (#1889)