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

Document charm commands #938

Merged
merged 9 commits into from
Jan 14, 2025
Merged

Document charm commands #938

merged 9 commits into from
Jan 14, 2025

Conversation

eaudetcobello
Copy link
Contributor

@eaudetcobello eaudetcobello commented Jan 8, 2025

Document charm commands

This PR documents the k8s charm commands.

@eaudetcobello eaudetcobello requested a review from a team as a code owner January 8, 2025 20:40
Copy link
Contributor

@HomayoonAlimohammadi HomayoonAlimohammadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @eaudetcobello, thanks a lot! Left some minor comments.

docs/src/charm/reference/commands.md Outdated Show resolved Hide resolved
docs/src/_parts/commands/charm/get-kubeconfig.md Outdated Show resolved Hide resolved
docs/src/_parts/commands/charm/get-kubeconfig.md Outdated Show resolved Hide resolved
@louiseschmidtgen louiseschmidtgen changed the title document charm commands Document charm commands Jan 9, 2025
Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Etienne! Let's add a little bit of context here and there.

docs/src/charm/reference/commands.md Outdated Show resolved Hide resolved
docs/src/_parts/commands/charm/pre-upgrade-check.md Outdated Show resolved Hide resolved
docs/src/_parts/commands/charm/pre-upgrade-check.md Outdated Show resolved Hide resolved
docs/src/_parts/commands/charm/get-kubeconfig.md Outdated Show resolved Hide resolved
docs/src/_parts/commands/charm/get-kubeconfig.md Outdated Show resolved Hide resolved
@bschimke95
Copy link
Contributor

@eaudetcobello should we close this PR or do you plan to refactor it to link to Charmhub?

@eaudetcobello
Copy link
Contributor Author

@bschimke95 Thanks for the nudge, I linked to charmhub.

Copy link
Member

@mateoflorido mateoflorido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think referring to actions as commands might cause confusion in the long run. I know we want to merge this with the commands page from the snap, but that feels a bit odd. @nhennigan do you think we should at least clarify this on the page, or completely change the naming to better reflect the charm mechanism?

docs/src/charm/reference/commands.md Outdated Show resolved Hide resolved
Copy link
Contributor

@addyess addyess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍨 Yum. Let's go

@nhennigan
Copy link
Contributor

I think referring to actions as commands might cause confusion in the long run. I know we want to merge this with the commands page from the snap, but that feels a bit odd. @nhennigan do you think we should at least clarify this on the page, or completely change the naming to better reflect the charm mechanism?

I think renaming the page makes the most sense. I was unaware of the difference in naming here. We can look at combining at a later stage. For now, we should go with what makes the most sense under the charm tree. Thanks for bringing it up. After the name change / title change it is good to go thanks Etienne 😸

@eaudetcobello eaudetcobello enabled auto-merge (squash) January 14, 2025 21:34
Copy link
Member

@mateoflorido mateoflorido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just check the nit before merging

docs/src/charm/reference/actions.md Outdated Show resolved Hide resolved
@eaudetcobello eaudetcobello merged commit 81650b7 into main Jan 14, 2025
6 checks passed
@eaudetcobello eaudetcobello deleted the KU-2425 branch January 14, 2025 21:48
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.

7 participants