-
Notifications
You must be signed in to change notification settings - Fork 394
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
Misc. updates #1615
Misc. updates #1615
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
||
- Not enough isolation/granularity - commands like `dvc metrics diff`, `dvc dag` | ||
and others by default dump all the metrics, all the pipelines, etc. | ||
- By default, DVC commands like `dvc checkout` and `dvc repro` explore the whole |
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.
still, case with not isolated/granular dvc metrics diff
seems a bit different than dvc pull
- one is mostly about performance, second is about actual output - might resonate with different people.
It's fine though to keep it this way. Not a strong one.
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. I changed the last sentence to "This can produce undesirable results and/or be inefficient for large monorepos." to better capture both cases.
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.
yep, but this is a good example of what I meant with implicit vs explicit. undesirable results
sounds and reads cool, and one paragraph looks better for sure, but it almost does not mean anything to a reader .. it's hard (or at least takes an effort) to get an example of what does it mean.
again, not a big deal in this case, just want to further explain my thought-process on being explicit 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 see. OK, split into 2 bullets for clarity in be5bc6d.
|
||
```dvc | ||
$ cd project-A | ||
$ dvc pull | ||
project-A # full DVC + Git repo |
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.
why do we need this comment?
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.
Hmmm they are informative. Changed to git init && dvc init
in cac92e5. WDYT?
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.
so, that's probably what I don't understand - if we put them they highlight importance of something to my mind, I spend a cycle thinking why did author put a comment here - is .git important in this case? does it change anything?
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.
They do intent to highlight exactly what we mean by nested or not-nested --subdir projects:
Not nested: single-level DVC projects in a Git repo.
Nested: multi-level DVC projects inside (DVC projects inside) DVC/Git repo
This could be also just undetstood by looking at the directories shown (.git, .dvc) but it's not as obvious).
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.
.git still not important here to my mind unless I'm missing something. Nesting would work with --no-scm
the same way effectively.
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'm removing these block examples completely for now, and extracting to a following PR...
that even if the project is tracked by Git, or if Git is initialized in it | ||
later, DVC will keep operating detached from Git in this project. | ||
|
||
> Note that like with nested <abbr>repositories</abbr> and `--subdir` projects, |
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.
so, if behavior is the same, do we actually need this note?
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.
Ieally not but yes in the way things are structured now in this ref. This isolation and nested situation got too complicated... I should probably make a separate sub-section to cover it after explaining all the 3 kinds of projects (typical or git-enabled, --subdir, --no-scm).
Can we extract to another issue/PR and get this merged maybe? I have some other changes I also want to throw in on another regular updates PR 😬
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 remove it then, create a ticket and discuss the changes in advance?
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.
Removed in 9c3ba77. I have all the text already in another branch though so I'll just open a draft PR to discuss, if you don't mind... ⌛
OMG sorry I linked the other PR to this one 😋 Reopening... |
I've approved it already a bit ago. Merging ... |
Thanks |
* cmd: review repro examples per #1572 (comment) * cmd: fic tyupo in get-url * cmd: updates to repro per #1572 (review) * cmd: rewrite repro -P desc rel #1572 (review) * cmd: simplified and generalize repro targets desc and DVC file mention per #1572 (comment) and #1572 (comment) * cmd: minor update for repro desc wording per #1572 (comment) * term: don't use "synchronize" in the context of checkout * cmd: rewrite Downstream example and added info for sequential execution of stages * Update content/docs/command-reference/repro.md * Update content/docs/command-reference/repro.md * Update content/docs/command-reference/repro.md * cmd: Updated Downstream example * Update content/docs/command-reference/repro.md * repro: Updated Downstream example * Update content/docs/command-reference/repro.md * cmd: updated last para for the description of --downstream and improved formatting * cmd: review language of init --subdir * term: revuew usage of "granular", esp. around init --subdir * repro.md: updated Downstream example * cmd: improve init --subdir explanation * cmd: add info about nested subrepos to init * cmd: fix -P option desc. per #1615 (review) * cmd: improve explanation on how --subdir affects commands per #1615 (review) * cmd: simplify nested structures explanation in init per #1615 (comment) * guide: add note aboud `cp` not being a download in external deps per #1643 (review) * cmd: add note about what --cwd means to repro per iterative/dvc#4292 (comment) * guide: nvmd! removing that note in external deps per 42b670f#r41087633 * Update content/docs/command-reference/repro.md * Update content/docs/command-reference/repro.md * Update content/docs/command-reference/repro.md * Update content/docs/command-reference/repro.md * Update content/docs/command-reference/repro.md * cmd: more small updates to init * Update content/docs/command-reference/repro.md * Update content/docs/command-reference/repro.md * Restyled by prettier * cmd: rewrap metrics diff usage paragraph * term: remove "just" from -j desc in 3 refs per eda27fc * cmd: add command examples to init --subdir use cases per #1615 * cmd: explain nested repo and projects of all kinds outside of --subdir per #1615 (review) * cmd: remove bold names to nested and not-nested structure examples in init --subdir per #1615 (review) * cmd: standardize --jobs option in all refs per #1615 (review) et al. * cmd: add speed note to --jobs desc in all refs. per #1615 (review) * cmd: change versioning command example in init per #1615 (review) * cd: change repo comments in init --subdir examples for #1615 (review) * cmd: improve note on DVC submodules a little for #1615 (review) * cmd: better explain why isolation is important in --subdir bullet per #1615 (review) * cmd: split last --subdir cases explicitly as 2 bullets per #1615 (comment) * cmd: remove most notes and code block examples about nesting projects/repos in init per #1615 (review) * cmd: add basic text about nesting to init * cmd: revert nested block examples back into --subdir per #1661 (review) * cmd: remove new section about nesting to revert more Co-authored-by: sarthakforwet <[email protected]> Co-authored-by: Restyled.io <[email protected]>
Triggered as out-of-scope from other tickets/PRs
--jobs
option description in all refs.