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

[PROPOSAL] Adopt style guide / lint tools code #245

Closed
dblock opened this issue Apr 16, 2024 · 1 comment · Fixed by #260
Closed

[PROPOSAL] Adopt style guide / lint tools code #245

dblock opened this issue Apr 16, 2024 · 1 comment · Fixed by #260
Labels
enhancement New feature or request

Comments

@dblock
Copy link
Member

dblock commented Apr 16, 2024

What/Why

What are you proposing?

I was trying to add some TypeScript code and started typing camelCase methods. Then I noticed that we have adopted of a Ruby-formatting/style in tools such as https://github.com/opensearch-project/opensearch-api-specification/blob/main/tools/merger/OpenApiMerger.ts with snake_case methods and ClassNames. This was probably intended. I propose we codify this, adopt a specific/default TypeScript style guide, and add a linter for it.

What users have asked for this feature?

Future contributors.

What problems are you trying to solve?

Introduce some rules before someone like myself contributes code that looks different.

What is the developer experience going to be?

CI will fail if contribution doesn't fit the style guide and linter doesn't pass.

@dblock dblock added enhancement New feature or request and removed untriaged labels Apr 16, 2024
@nhtruong
Copy link
Collaborator

I'm bias for spending years writing Ruby code, but I find snake_case for method names a lot more readable, especially on longer names. Even though the tools are written in TypeScript, where CamelCase is the convention, I'd still vote for snake_case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants