-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various variables do not use proper camelCase for instances of the words “ID” and “UID” #6741
Comments
eslint's camelcase rule prefers EDIT: That is to say, eslint should probably be trusted as an authoritative source on this, and this should not be changed. |
@aduth @youknowriad Do either of you have an opinion on this? |
Previously: #2511 |
My latest personal leanings based on consistency with the broader ecosystem at large is to capitalize acronyms, and camelcase everything else. Examples:
|
@aduth To avoid bikeshedding again, I suggest we just go for it. Can we enforce these in linting rules? |
@danielbachhuber this will revert this merge then, correct??? |
@robertdevore Yep. But we need a final say on the direction we want to head. |
Not as proposed in #6741 (comment), since it would rely on an understanding of what is and isn't an acronym or abbreviation. If we subscribed to the idea that all things be camel-cased, we could set a rule that a variable must begin with a lower-cased letter and, if a capital letter is ever encountered, it can't be followed by anything other than a lowercase letter. We'd also need to consider things like classes, where If we want to avoid bikeshed, here's my (opinionated) suggestions:
|
I find @aduth's suggestion fine; it's what I'm used to in other JS projects. For |
Raised again in Slack to be addressed: https://wordpress.slack.com/archives/C02QB2JS7/p1530544700000001 I don't love the idea of an exception, as it seems harder to enforce consistently. There's been some struggle on the naming of a block's It's for this reason I'm wondering if we could be more explicit here while eliminating the chance for ambiguity on capitalization, calling it instead a |
|
#7669 addressed this. |
The issue
In some of the files in the code, I noticed that
firstUid
,lastUid
,noticeId
andupdatedId
See:
https://github.com/WordPress/gutenberg/search?utf8=%E2%9C%93&q=firstUid
https://github.com/WordPress/gutenberg/search?utf8=%E2%9C%93&q=lastUid
https://github.com/WordPress/gutenberg/search?utf8=%E2%9C%93&q=updatedId
https://github.com/WordPress/gutenberg/blob/63abd32d9b5cb809ad71167f209dd7a6d0a79a3f/editor/components/block-mover/index.js
https://github.com/WordPress/gutenberg/blob/63abd32d9b5cb809ad71167f209dd7a6d0a79a3f/editor/store/test/actions.js
https://github.com/WordPress/gutenberg/blob/63abd32d9b5cb809ad71167f209dd7a6d0a79a3f/editor/store/actions.js
https://github.com/WordPress/gutenberg/blob/c45d6bd5380f2e7a9a5d9ce82b3fcb845fb715b4/editor/store/reducer.js
https://github.com/WordPress/gutenberg/blob/c45d6bd5380f2e7a9a5d9ce82b3fcb845fb715b4/editor/store/test/reducer.js
https://github.com/WordPress/gutenberg/blob/fb804e28585be0d7346953707c03080bfa5d604d/editor/store/effects.js
https://github.com/WordPress/gutenberg/blob/63abd32d9b5cb809ad71167f209dd7a6d0a79a3f/editor/store/test/effects.js
From #6742:
Assuming I understand camelCase properly, “ID” and “UID” should always be entirely capitalized unless it is the first word in a name, in which case it should be entirely lowercase.
Expected behavior
“ID” and "UID" should be correctly capitalized in all functions, variables, etc. according to camelCase standards.
Related Issues and/or PRs
#6685
#6742
The text was updated successfully, but these errors were encountered: