-
Notifications
You must be signed in to change notification settings - Fork 687
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
Clients/js/refactor cmds & CLI docs generator #3182
Merged
evan-gray
merged 28 commits into
wormhole-foundation:main
from
AlberErre:clients/js/refactor-cmds
Jul 17, 2023
Merged
Clients/js/refactor cmds & CLI docs generator #3182
evan-gray
merged 28 commits into
wormhole-foundation:main
from
AlberErre:clients/js/refactor-cmds
Jul 17, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlberErre
requested review from
kcsongor,
kev1n-peters and
evan-gray
as code owners
July 9, 2023 09:52
@aadam-10 @kcsongor @evan-gray @a5-pickle @heyitaki just created this MR to extract the refactor done on #2631 regarding cmds modules & auto-generated documentation 👀 This way is easier to review 🎉🏄♂️ |
barnjamin
reviewed
Jul 10, 2023
barnjamin
reviewed
Jul 10, 2023
barnjamin
reviewed
Jul 10, 2023
barnjamin
previously approved these changes
Jul 10, 2023
@barnjamin just synced with could you approve it again? ✅ thanks! |
barnjamin
approved these changes
Jul 13, 2023
kcsongor
approved these changes
Jul 17, 2023
bruce-riley
pushed a commit
that referenced
this pull request
Jul 17, 2023
* feat: update npm commands for Githun Actions CI * feat: add worm cli github actions * feat: save HTML test report as artifact * chore: update github action, show report correctly * fix: add missing CommandModule type * chore: rm unused import * fix: override auto-detected locale by OS system * feat: infere command modules on doc.ts & main.ts * feat: command args accepts an array of modules * fix: cmds must be outside main, breaks otherwise * fix: import CLI_COMMAND_MODULES outside of main * chore: add missing transfer command from README * chore: rm test branch dependencies * feat: extract info cmds into array const * chore: document command imports as list * chore: package.json spacing * chore: bump @types/yargs version * feat: cast correct array type YargsCommandModule[]
panoel
pushed a commit
that referenced
this pull request
Jul 19, 2023
* feat: update npm commands for Githun Actions CI * feat: add worm cli github actions * feat: save HTML test report as artifact * chore: update github action, show report correctly * fix: add missing CommandModule type * chore: rm unused import * fix: override auto-detected locale by OS system * feat: infere command modules on doc.ts & main.ts * feat: command args accepts an array of modules * fix: cmds must be outside main, breaks otherwise * fix: import CLI_COMMAND_MODULES outside of main * chore: add missing transfer command from README * chore: rm test branch dependencies * feat: extract info cmds into array const * chore: document command imports as list * chore: package.json spacing * chore: bump @types/yargs version * feat: cast correct array type YargsCommandModule[]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ℹ️ This PR exposes all CLI commands modules into a constant array called
CLI_COMMAND_MODULES
.Looks like all commands can be imported into a singe
.command()
call 👀, if we export these commands as a JS module 🎉More info about this approach can be found on the official documentation.
This refactor is needed to provide a single source of truth for all CLI commands. By this approach, we would be able to implement testing on CLI commands against auto-generated documentation, as pointed out here -> #2631 (comment)
Also, this PR refactors
clients/js/src/doc.ts
documentation generator:english
locale has been added (without this, documentation is generated on the language of the OS machine, it was generated in spanish in my case).CLI_COMMAND_MODULES
variable has been used to derive commands. This avoids maintaining multiple commands lists over the CLI, now we have a single source of truth.