Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refactor: Apply lint rule cred-class-field-display-name-miscased #4762

Merged
merged 4 commits into from
Nov 30, 2022
Merged

refactor: Apply lint rule cred-class-field-display-name-miscased #4762

merged 4 commits into from
Nov 30, 2022

Conversation

ivov
Copy link
Contributor

@ivov ivov commented Nov 29, 2022 •
edited
Loading

@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team node/improvement New feature or request labels Nov 29, 2022
@netroy
Copy link
Member

netroy commented Nov 29, 2022

instead of making an exception for E-goi, should we change the rule to enfore the name that you get after removing certain characters like -?

@ivov
Copy link
Contributor Author

ivov commented Nov 30, 2022

titleCase in the plugin capitalizes all words except articles and prepositions and conjunctions, whether hyphenated or not. This matches traditional rules for casing of hyphenated nouns and adjectives in titles, which makes me think hyphenated brand names in nodes-base (rare as they are, no other example) are likely to follow the pattern, making E-goi an exception, but hard for me to say. Maybe @StarfallProjects knows better.

@StarfallProjects
Copy link
Contributor

StarfallProjects commented Nov 30, 2022 •
edited
Loading

I think Ivan's approach will be right in most cases. The site he links describes a common style convention for title case headings (I think it may be drawn from the Chicago Manual of Style? Don't quote me on that though).

Conventions vary depending on what style guide you follow. Are we basing our UI copy styles on any particular style guide? Worth noting that a lot of popular tech style guides (i.e. guides focused on digital rather than print, including Google's and Microsoft's) avoid title case altogether, but that's a separate issue: main thing is we're consistent, and matching brand names exactly.

@ivov
Copy link
Contributor Author

ivov commented Nov 30, 2022

Are we basing our UI copy styles on any particular style guide?

Mostly David's indications on top of Jan's preexisting choices. David is not a fan of title case either but we kept it because it was so prevalent.

@ivov ivov merged commit cc7f8fd into master Nov 30, 2022
@ivov ivov deleted the n8n-5317-apply-lint-rule-cred-class-field-display branch November 30, 2022 14:31
@n8n-assistant n8n-assistant bot added the Upcoming Release Will be part of the upcoming release label Nov 30, 2022
MiloradFilipovic added a commit that referenced this pull request Dec 1, 2022
@MiloradFilipovic
Merge branch 'master' into feature/undo-redo
7c22278 
* master:
  fix(editor): JSON view values can be mapped like keys (#4702)
  test: E2E NDV (#4712)
  fix: Issue building image with dependency name ending in .vue (no-changelog) (#4773)
  fix: Register community nodes as known nodes (no-changelog) (#4775)
  refactor: Codex updates for Item Lists, Markdown and Date & Time (#4772)
  refactor(editor): Patch ElementUI tooltip memory leak (#4769)
  refactor: Apply lint rule `cred-class-field-display-name-miscased` (#4762)
@janober
Copy link
Member

janober commented Dec 2, 2022

Got released with n8n@0.205.0

@janober janober removed the Upcoming Release Will be part of the upcoming release label Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@netroy netroy netroy approved these changes

Assignees
No one assigned
Labels
n8n team Authored by the n8n team node/improvement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ivov @netroy @StarfallProjects @janober