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

chore: adds ohmyzsh plugin for gaiad #2858

Closed

Conversation

tokamak-git
Copy link
Contributor

Description

Closes: #1331

#1331

adds ohmyzsh completion script with instructions for gaiad


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • Included the correct type prefix in the PR title
  • Targeted the correct branch (see PR Targeting)
  • Provided a link to the relevant issue or specification
  • Reviewed "Files changed" and left comments if necessary
  • Confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • Confirmed the correct type prefix in the PR title
  • Confirmed all author checklist items have been addressed
  • Confirmed that this PR does not change production code

@@ -0,0 +1,9 @@
# gaiad
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please move shell_completion folder to ./contrib folder of the gaia repo?

@tokamak-git
Copy link
Contributor Author

hey @MSalopek , any other changes required to merge this in?

Copy link
Contributor

@MSalopek MSalopek left a comment

Choose a reason for hiding this comment

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

hey @MSalopek , any other changes required to merge this in?

  1. I don't see any provider commands listed in the completions file. Is that by design?

  2. Instructions on how to update the completions when the CLI changes should be provided.

  3. I'm assuming the completions were generated? If so, can you provide the generation script?

created an action that builds and gerated the completion script for ohmyzsh plugin using the inbuilt cobra engine
updated makefile so as to make it easier to interact with this action and generate the script
@tokamak-git
Copy link
Contributor Author

1. I don't see any `provider` commands listed in the [completions file](https://github.com/cosmos/gaia/blob/6d65ab17a7a7401a4c5d1427fb545892153df4fa/contrib/shell_completion/_gaiad). Is that by design?

the script is auto-generated thus did not modify it so as to ensure reproducibility.

2. Instructions on how to update the completions when the CLI changes should be provided.
3. I'm assuming the completions were generated? If so, can you provide the generation script?

Have added a command in the MAKEFILE make gen-completion

have also added a github action that runs on main and release branches so as to automate the process of the script creation. Your input on the same would be appreciated.

name: Generate ohmyzsh completion
on:
push:
branches:
Copy link
Contributor

@mmulji-ic mmulji-ic Jan 3, 2024

Choose a reason for hiding this comment

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

Does this then update the script for every push to main/release branches? My preference would be to do this manually (in the release checklist) or add it to the release task. Do you have a preference @MSalopek ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mmulji-ic have removed the automation script so as to allow for manual generation and distribution of the script rather than relying on automation.

@@ -309,3 +309,12 @@ proto-swagger-gen:
@sh ./proto/scripts/protoc-swagger-gen.sh

.PHONY: proto-gen proto-doc proto-swagger-gen

Copy link
Contributor

@mmulji-ic mmulji-ic Jan 3, 2024

Choose a reason for hiding this comment

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

Running this target, produced a different contrib/shell_completion/_gaiad file. Should update that file for this PR. Want to see if its due to system differences or a build off an older commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @mmulji-ic have tested this on my local machine and it is indeed updating the _gaiad file rather than creating a new one.

Is there any specific step you are following? so that i can reproduce this on my machine?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @tokamak-git the file generated on my system was different, was wondering if this was system related or just an older generation of the completions file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it will update depending on what features the cli implements.

So it should have a different output as compared as I'm sure the branch your on has different cli features from the one generated on the pr.

@tokamak-git
Copy link
Contributor Author

Hey folks. are there any changes you would recommend to this?

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label May 20, 2024
@github-actions github-actions bot closed this May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gaiad completion script
3 participants