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.
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
docs: doc repro --glob and update targets arg info. #1983
docs: doc repro --glob and update targets arg info. #1983
Changes from 12 commits
ac71914
c78da03
a4f8404
f650ea6
955cc3d
25959a0
3c099b9
af8f7fc
9255866
8cd83c3
fbe65ab
a34e74d
822576a
9be4347
1c85228
474a2a6
9734ec6
6e87280
10aedde
ebb2feb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
all not relevant?
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.
This paragraph is redundant (with the new targets text a well as the first p in the Description) and slightly wrong (but that will be corrected for #2024). Removing it aims to make the Description shorter so that the whole doc doesn't grow too much as we introduced the targets text.
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.
But I can easily stash the change if you prefer and leave it for later. Lmk.
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.
Yes, let's make PRs focused if possible.
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.
I expect some changes here, but it probably should be only the paragraph about targets?
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.
OK, I reverted this. I'm afraid that if we just keep adding paragraphs without reviewing the context we'll end up with lots of redundancy and a text that may be unnecessary long.
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.
let's skip this I think?
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.
Yes! I would like that too but are you sure? The help output (and Synopsis above) mention .dvc files so if we don't explain it here, it'd be up to the user to assume what will happen.
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.
Synopsis is a copy/paste from DVC which should probably change, so, let's not complicate. The point is not cover everything to the very small detail - the point is to focus on important stuff and convey it in the simplest possible format (e.g. people don't read long texts, unless they want to dive in)- thus short self-explainable examples are better
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.
Agree. Removed info about .dvc file targets from this ref.
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.
not relevant?
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.
This is directly related to
repro targets
. It used to read "determines the stages ... by searching ... for stages". Isn't this a good opportunity to fix that? TBH I already have a mountain of stashed changes so anything I can't get into existing PRs will probably get lost until we happen to notice it again 🙁