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

Fixes OpenSearchISMPolicy crd kind in docs #741

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

jojovem
Copy link
Contributor

@jojovem jojovem commented Feb 26, 2024

Minor typo fix for the OpenSearchISMPolicy kind

Description

Minor typo fix for the OpenSearchISMPolicy kind

Minor typo fix for the OpenSearchISMPolicy kind

Signed-off-by: Gustavo Andrade Ferreira <jojovembh@gmail.com>
@jojovem
Copy link
Contributor Author

jojovem commented Feb 26, 2024

I dont know what is the pattern for CRD naming, but they are using capital S in some and lower case in others:

  • OpensearchActionGroup
  • OpenSearchCluster
  • OpensearchComponentTemplate
  • OpensearchIndexTemplate
  • OpenSearchISMPolicy
  • OpensearchRole
  • OpensearchTenant
  • OpensearchUserRoleBinding
  • OpensearchUser

@salyh
Copy link
Collaborator

salyh commented Mar 28, 2024

I would vote for capital 'S' like in OpenSearchISMPolicy for all new CRDs, and then, with the next major release, change all the other CRDs to also be camel cased (as a breaking change). Wdyt @idanl21 @dbason @swoehrl-mw @prudhvigodithi @jochenkressin @pchmielnik

@pchmielnik
Copy link
Collaborator

I would vote for capital 'S' like in OpenSearchISMPolicy for all new CRDs, and then, with the next major release, change all the other CRDs to also be camel cased (as a breaking change). Wdyt @idanl21 @dbason @swoehrl-mw @prudhvigodithi @jochenkressin @pchmielnik

I like the idea of using a capital 'S'. This approach promotes consistency and clarity in naming conventions, which is beneficial for long-term maintenance and usability.

@prudhvigodithi
Copy link
Collaborator

prudhvigodithi commented Apr 2, 2024

I would go for capital S and once merged I assume this would be a breaking change for folks who have applied OpensearchISMPolicy right ?

@prudhvigodithi
Copy link
Collaborator

Should we push this change to v2.6.0 #776 release?
WDYT @idanl21 @dbason @swoehrl-mw @prudhvigodithi @jochenkressin @pchmielnik @bbarani

@swoehrl-mw
Copy link
Collaborator

@prudhvigodithi

I would go for capital S and once merged I assume this would be a breaking change for folks who have applied OpensearchISMPolicy right ?

This PR is only a doc change, in the code the capital S is already correct, so no breaking change. That also means it makes no difference if it is merged before or after the release.

@prudhvigodithi
Copy link
Collaborator

Thanks then can we go ahead with the PR?

@swoehrl-mw swoehrl-mw merged commit 608c678 into opensearch-project:main Apr 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants