Skip to content

remove: updated to match with core repo#1497

Merged
jorgeorpinel merged 8 commits into
treeverse:masterfrom
nik123:fix1490
Jun 27, 2020
Merged

remove: updated to match with core repo#1497
jorgeorpinel merged 8 commits into
treeverse:masterfrom
nik123:fix1490

Conversation

@nik123
Copy link
Copy Markdown
Contributor

@nik123 nik123 commented Jun 24, 2020

Fixes #1490

* `dvc remove` description updated according to implementation (see treeverse/dvc#4094 for context)

* `--outs` description updated according to implementation.

* `.gitignore` described in examples.

* Removed extra params: `-p`, `-f`.

* Removed shorthand for `--outs` (i.e. `-o`).

* Updated command description.
@shcheklein shcheklein requested a review from jorgeorpinel June 24, 2020 21:39
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Comment thread content/docs/command-reference/remove.md Outdated
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nik123 ! I left a bunch of suggestions. I also think we could use another example because we only have one for a .dvc file, what about removing a stage from dvc.yaml?

Also, not sure what your role is currently so totally OK if this goes beyond the scope of what you can commit to, I can finish it. Just lmk 🙂

nik123 and others added 2 commits June 26, 2020 13:15
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@nik123

This comment has been minimized.

@jorgeorpinel

This comment has been minimized.

nik123 and others added 2 commits June 26, 2020 17:18
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@nik123
Copy link
Copy Markdown
Contributor Author

nik123 commented Jun 26, 2020

As far as understand, there were 2 issues with examples in original code. So I'll try address them both:

  • 1. Example with dvc-files is too complex

I've updated example and now it doesn't require reader to know about linux pipes or wc -l command. I'm not sure it became better though so please, check it out. May be we'll have to fall back to previous version.

  • 2. We need "remove stage" example with dvc.yaml file

Right now I can't come up with nice, short and simple example for dvc.yaml. If you have an idea for one or think that dvc.yaml example shouldn't necessarily be addressed in this PR, I'm totally fine with it.

@nik123 nik123 requested a review from jorgeorpinel June 26, 2020 10:50
Comment thread content/docs/command-reference/remove.md Outdated
@jorgeorpinel jorgeorpinel merged commit 3ab081a into treeverse:master Jun 27, 2020
@jorgeorpinel
Copy link
Copy Markdown
Contributor

No worries, I'll address #2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cmd ref for dvc remove is out of date

3 participants