Skip to content
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

term: DVC-file -> .dvc file from Utkarsh work (2nd chunk) #1403

Closed
wants to merge 1 commit into from

Conversation

jorgeorpinel
Copy link
Contributor

Continuation of #1372 (review)

Cc @utkarshsingh99 sorry, I'm basically stealing your work a little here. Need to move things along and the git operations were a little complex.

@calibre-analytics
Copy link

Your trial has ended

Choose a plan to resume monitoring your Sites and Pull Requests. If you need help, check the Manage Your Plan and Test Usage guide.

@shcheklein shcheklein temporarily deployed to dvc-landing-1-0-ter-dvc-veltgo June 8, 2020 09:01 Inactive
targets DVC-files to remove.
targets `.dvc` files to remove.
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jun 8, 2020

Choose a reason for hiding this comment

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

@efiop we need to do this same for the core repo when possible please. See iterative/dvc/issues/3960

Copy link
Contributor

Choose a reason for hiding this comment

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

@jorgeorpinel Wait, that looks wrong. It should be "stages or .dvc files to remove". It works with dvc.yaml too.

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Jun 8, 2020

Choose a reason for hiding this comment

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

I was assuming remove only works with .dvc files like move. OK, great to know, so let's go with this:

Suggested change
targets DVC-files to remove.
targets `.dvc` files to remove.
targets stages (found in `dvc.yaml`) or `.dvc` files to remove.

Cc @utkarshsingh99

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking of not to mention this yet, only after we release docs for run.

Copy link
Member

Choose a reason for hiding this comment

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

remove only has --outs flag now, and 0.94's remove and 1.0's remove behavior are completely different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, good to know @skshetry . Please open a separate issue or PR from the core repo PR where remove is changed (usual process). This PR is about updating "DVC-file" terminology. Thanks

Copy link
Contributor Author

@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.

@utkarshsingh99 OK here's the plan! I reviewed half of the files here so far. Please copy this branch to your fork and RE-OPEN the PR from there (so that you can contribute to it because you won't be able to commit here). Then address by review 👇 and ping me so we can continue it there.

I'll just close this draft PR when that happens. Thanks

@jorgeorpinel
Copy link
Contributor Author

A couple questions to Ruslan are still open but I'll close this now in favor of #1408

@jorgeorpinel
Copy link
Contributor Author

couple questions to Ruslan

Actually just #1403 (review) now, and it's more of a comment/reminder, not a question.

Abandoning this ship! ⚓

@jorgeorpinel jorgeorpinel deleted the 1.0/ter/dvcfile/basics-2 branch June 23, 2020 04:15
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.

5 participants