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.

Sign up for GitHub

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

Jump to bottom

run: use -f/--force to overwrite, fix msg #4011

Merged
merged 1 commit into from
Jun 10, 2020
Merged

run: use -f/--force to overwrite, fix msg #4011

merged 1 commit into from
Jun 10, 2020

Conversation

skshetry
Copy link
Member

@skshetry skshetry commented Jun 10, 2020 •
edited by jorgeorpinel
Loading

Also, makes fixes for --overwrite rename to --force
and also remove remove_with_prompt which was only being
used with force=True anyway.

Fix #4003

iterative/dvc.org#1255

  • ❌ I will check DeepSource, CodeClimate, and other sanity checks below. (We consider them recommendatory and don't expect everything to be addressed. Please fix things that actually improve code or fix bugs.)

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

@skshetry skshetry requested review from pmrowla, efiop and pared June 10, 2020 13:41
@skshetry skshetry self-assigned this Jun 10, 2020
skshetry
skshetry commented Jun 10, 2020
View reviewed changes
dvc/dvcfile.py
@@ -148,19 +146,6 @@ def dump(self, stage, **kwargs):
dump_yaml(self.path, serialize.to_single_stage_file(stage))
self.repo.scm.track_file(self.relpath)

def remove_with_prompt(self, force=False):
Copy link
Member Author

Choose a reason for hiding this comment

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

This was only being used as force=True making it equivalent to .remove().

@skshetry
run: use -f/--force to overwrite, fix msg
21e65dd 
Also, makes fixes for `--overwrite` rename to `--force`
and also remove `remove_with_prompt` which was only being
used with `force\=True` anyway.

Fix iterative#4003
@jorgeorpinel jorgeorpinel mentioned this pull request Jun 10, 2020
Closed
20 tasks
@efiop efiop merged commit 4042d52 into iterative:master Jun 10, 2020
@skshetry skshetry deleted the fix-4003 branch June 10, 2020 17:19
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jun 11, 2020 •
edited
Loading

Hi, how is this expressed in iterative/dvc.org/issues/1255 ? I'm not seeing it, thanks.

@jorgeorpinel
Copy link
Contributor

p.s. probably best to open an individual docs issue. That epic is for very specific 1.0 discrepancies. Let's try to keep the same process as always.

@skshetry
Copy link
Member Author

@jorgeorpinel, many of the commands were changed for 1.0. And, this is a very 1.0 discrepancy. And, it's not too important for get-started.

@efiop
Copy link
Contributor

efiop commented Jun 11, 2020

@jorgeorpinel Same as with my -f change, there are too many conflicting docs to edit, so added a checkbox to that epic.

@jorgeorpinel jorgeorpinel mentioned this pull request Jun 12, 2020
Merged
1 task
@jorgeorpinel
Copy link
Contributor

I don't see a checkbox for this in the epic actually. But can we please follow the regular process 🙏 sorry if the epic was confusing.

For this one I will just incorporate in iterative/dvc.org/pull/1420 since I'm just getting started. Thanks

@jorgeorpinel jorgeorpinel mentioned this pull request Jun 12, 2020
Merged
1 task
@shcheklein
Copy link
Member

For this one I will just incorporate in iterative/dvc.org/pull/1420

not a good idea I think, let's try to keep iterative/dvc.org/pull/1420 focused on dvc run only and do it faster, wdyt?

@jorgeorpinel
Copy link
Contributor

Sure. But this PR is now part of 1.0 pre-release dvc run and I'm just getting started with updating that cmd ref description and examples, so this behavior will already be included as I use the cmd to confirm everything. That's why it should be covered already in iterative/dvc.org/pull/1420

@efiop
Copy link
Contributor

efiop commented Jun 12, 2020 •
edited
Loading

I don't see a checkbox for this in the epic actually.

@jorgeorpinel There was: get rid of -f in add/run/import(-url) per iterative/dvc#4004 and iterative/dvc#4011, you can see it in the history. Looks like you've removed it in the past two days.

@jorgeorpinel
Copy link
Contributor

I extracted that other one you added into its own issue but it wasn't for this, unless I don't understand how "get rid of -f in run" per #4004 = "run: use -f/--force to overwrite" per this PR.

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

@efiop efiop efiop approved these changes

@pmrowla pmrowla Awaiting requested review from pmrowla

@pared pared Awaiting requested review from pared

Assignees

@skshetry skshetry

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

run: improve stage exists error message to hint how to overwrite (edit) it
4 participants
@skshetry @jorgeorpinel @efiop @shcheklein