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.

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

feat(linter): implement noLowerCaseEnum #4031

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eMerzh
Copy link
Contributor

@eMerzh eMerzh commented Sep 21, 2024

Summary

  • it's globally accepted that enums should be in uppercase, although not a "spec requirement", some other tools have this lint.

please be indulgent, i'm a very noob in rust /o\

also there are no linked issue bc i'm not sure which one i should create.

Test Plan

see tests

@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter A-Diagnostic Area: diagnostocis labels Sep 21, 2024
Copy link

codspeed-hq bot commented Sep 21, 2024

CodSpeed Performance Report

Merging #4031 will not alter performance

Comparing eMerzh:gql_all_caps_enum (ac5384e) with main (4848994)

Summary

✅ 97 untouched benchmarks

@minht11
Copy link
Contributor

minht11 commented Sep 21, 2024

I think could be handled by https://biomejs.dev/linter/rules/use-naming-convention/#typescript-enum-names ?


If you are planning on working issue yourself, you can open straight up and someone will assign you, but you do not need to wait for that. Though discussion can help to make sure you are going into right direction.

If you do not plan working on it but want it implemented:

  1. For existing rules use ✅ Linter rules from other sources #3
  2. If rule is completely unique open discussiong entry.

@eMerzh
Copy link
Contributor Author

eMerzh commented Sep 21, 2024

mmh i have no idea for the ts enums 🤔 even if those are Graphql enums? 🤔 and if you want differents rules between gql & ts?

@dyc3
Copy link
Contributor

dyc3 commented Sep 21, 2024

I'll defer to somebody else for what the name for this rule should be. There should also be a changelog entry for this.

@eMerzh
Copy link
Contributor Author

eMerzh commented Sep 21, 2024

thx for the quick review, I've added the changelog entry

@minht11
Copy link
Contributor

minht11 commented Sep 22, 2024

I think originally the idea was that multiple languages can have the same rule. Like for example json language has useSortedKeys, the same rule could be implemented for javascript as well. noLowerCaseEnum could be part of useNamingConvention but for graphql, configuration would be separate for each language. As far as I know currently there are no rules like that, so there are a lot of unanswered questions.

@github-actions github-actions bot removed the A-Changelog Area: changelog label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants