-
Notifications
You must be signed in to change notification settings - Fork 393
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
cmd ref: add list command #1021
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guro, looks good. PTAL and address some comments that make sense to you. Otherwise it is in a good shape that we can take and iterate on it. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @JIoJIaJIu ! We'll take this over π
|
I rewrote the fist half @shcheklein. Please check when you can and I'll address your feedback at that time along with a review of the examples. Hoping we also get answers to the few questions I added above by then. Thanks! |
This comment has been minimized.
This comment has been minimized.
@jorgeorpinel @JIoJIaJIu looks great! @jorgeorpinel should we also update the data registry to include the list? |
takes too long ... merging this, please make other fixes as a regular update |
Follows #967
Relates:
β Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.
π Please make sure to mention
Fix #issue
(if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.Please chose to allow us to edit your branch when creating the PR.
Thank you for the contribution - we'll try to review it as soon as possible. π