-
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: document checkout displaying changes #1054
Conversation
This comment has been minimized.
This comment has been minimized.
20ec6ac
to
c4f968c
Compare
c4f968c
to
6da4ccc
Compare
I tried adding outputs of |
@skshetry could we at least remove existing wrong outputs (if any)? Or all of them silent now? |
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.
2 small things but I also think we should def. review all the instances of dvc checkout
usage in sample console blocks and remove any outdated output or, if relevant, update it with the current version's output.
We could take this over if needed.
@skshetry @jorgeorpinel what is the status of this, guys? |
@shcheklein idk but I just committed my suggestions since this has been waiting for a long time now. Should we take it over? Or we can merge it and open a separate issue for
|
@jorgeorpinel, @shcheklein, sorry, this got lost in my notifications. AFAICT the docs do not have any outputs on checkouts (and, as checkouts were silent before), so, we don't need to change other parts. |
@jorgeorpinel yes, let's take this over and update checkout outputs. I would not do this everywhere - only in places where |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
could we at least remove existing wrong outputs (if any)? Or all of them silent now?
There's already no output samples for dvc checkout
in code blocks... We can merge this!
I did merge master since this branch is kind of old, and updated the main example in the checkout cmd ref (including the actual command output). |
This comment has been minimized.
This comment has been minimized.
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.
looks good overall, see a few comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Done in a819927 (for all use cases) Also solved merge conflicts now. |
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.
just one minor typo
❗ 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. 🙏
iterative/dvc#3401 is under review. I have kept this docs to minimum, as the PR is more in relation with UI than any behavioral change. But. I see that, the behavior that it would checkout silent if there's an error was mentioned, which I have adjusted a bit. Also, some places look good to show outputs of
dvc checkout
.