Docs for new command dvc check-ignore#1629
Conversation
| ``` | ||
|
|
||
| If the `--details` option is used, a series of lines are printed using this format: | ||
| `<source> <COLON> <linenum> <COLON> <pattern> <HT> <pathname>` |
There was a problem hiding this comment.
| `<source> <COLON> <linenum> <COLON> <pattern> <HT> <pathname>` | |
| `.dvcignore:<line number>:<pattern> <HT> <target path>` |
- Why
<source>? It's always.dvcignoreright? Or because of nested ones? If so maybe use<path/to/>.dvcignore - What's
<HT>? Please replace with actual character.
There was a problem hiding this comment.
@karajan1001 actually I'm not sure what the answers are here, can you check it? Thanks
There was a problem hiding this comment.
@jorgeorpinel,
Not always .dvcignore, might be data/.dvcignore as well. We didn't restrict .dvcignore to the root of the repo.
There was a problem hiding this comment.
OK so probably:
| `<source> <COLON> <linenum> <COLON> <pattern> <HT> <pathname>` | |
| `<path/to/.dvcignore>:<line_num>:<pattern> | <target_path>` |
HT = horizontal tab, right?
UPDATE: Committed.
jorgeorpinel
left a comment
jorgeorpinel
left a comment
There was a problem hiding this comment.
Thanks @karajan1001, great start! I've rewritten it to the point it's almost mergeable but there are some questions I left here and in treeverse/dvc/pull/4282 that may affect the end result.
|
@jorgeorpinel @karajan1001 the PR https://github.com/iterative/dvc/pull/42826 got merged today. what is the status of this? ready to merge as well? |
shcheklein
left a comment
shcheklein
left a comment
There was a problem hiding this comment.
Awesome!! 😎 Please check some comments to address some minor changes + some technical stuff related to the engine.
|
I committed a bunch of my suggestions but there's still one or two questions pending from the old ones (plus Ivans comments) and the formatting needs fixing (sorry, may have been my bad) — we can just merge the Restyled PR though so that's not a huge deal. |
jorgeorpinel
dismissed
shcheklein’s stale review
I'll take the liberty to merge this as-is since I addressed all your feedback. Another PR on this ref is coming soon so we can fix anything else in there...
| according to the [`.dvcignore` file](/doc/user-guide/dvcignore) (if any). The | ||
| ones that are ignored indeed are printed back. | ||
|
|
||
| > Note that your shell may support path wildcards such as `dir/file*` and these |
There was a problem hiding this comment.
same - it's fine to keep it here, but that what I would expect anyways since multiple targets are supported. A bit not consistent since we don't make notes like this in other commands.
There was a problem hiding this comment.
Oh I see you found my note. Yeah any other commands where this note would be especially relevant? add/commit/remove? status/repro? fetch/pull/push? metrics/plots? (un)freeze?
There was a problem hiding this comment.
I guess best to just remove it too...
|
|
||
| ## Syntax | ||
|
|
||
| The same as for [`.gitignore`](https://git-scm.com/docs/gitignore). |
There was a problem hiding this comment.
I think that was useful. Also, we do quite a bit of changes not related directly to the command, @jorgeorpinel :)
There was a problem hiding this comment.
Yes, sorry. But had to read the whole thing to find the best places to link to the command and did copy edits along the way.
This section was repetitive since the format (and templates) is already linked to from the bullets above. It was also an entire H2 section for 1-line — looked funny once rendered.
Should I restore it and move all the syntax/format and templates info here? (remove it from the bullet under How it works)
shcheklein
left a comment
shcheklein
left a comment
There was a problem hiding this comment.
Looks great 👍 Thanks 🙏 @karajan1001 @jorgeorpinel . Some comments to improve in the next revision.
Also, would be great eventually to use a bit simpler examples with some real case -e.g. ignore pycache - it applies to all dvcignore documents. I guess we have some tickets already.
Just #519 |
* cmd: remove unnecessary commas in get and import * cmd: fix typo in add * cmd: remote copy edits per #1617 (comment) * guide: .dvcignore copy edit * cmd: init copy edits * clarify about dirs in import -o * cmd: review get -o desc * dvcignore: updates to guide and check-ignore ref. per #1629 (review) et al. * cmd: update check-ignore -n per treeverse/dvc#4323 (comment) * cmd: fix get.import -o descriptions per #1673 (review) and #1673 (review) * cmd: copy edits to remote add/modify
3 participants
Fixes #1628, Only a draft PR to explain how it works.
❗ 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 choose 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. 🙏