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: replace 'DVC-file' and 'stage file' with correct terms (#3960) #5280

Merged
merged 1 commit into from
Feb 5, 2021
Merged

Conversation

DiPaolo
Copy link
Contributor

@DiPaolo DiPaolo commented Jan 17, 2021

Fixes #3960

Documentation on dvc.org was already updated before.

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

Copy link
Contributor

@pmrowla pmrowla left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, just a few minor suggestions.

Also, this needs a rebase now after some of the changes in master.

.pylintrc Outdated Show resolved Hide resolved
dvc/command/repro.py Outdated Show resolved Hide resolved
dvc/dvcfile.py Outdated
@@ -65,7 +65,7 @@ def is_lock_file(path):
def check_dvc_filename(path):
if not is_valid_filename(path):
raise StageFileBadNameError(
"bad DVC-file name '{}'. DVC-files should be named "
"bad DVC file name '{}'. DVC files should be named "
"'Dvcfile' or have a '.dvc' suffix (e.g. '{}.dvc').".format(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can get rid of the Dvcfile part of this error message entirely since we are completely dropping single-stage support in 2.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would leave it as it is for now. Later it can be changed closely to 2.0. Anyway, I'm not so informed about 2.0 specific, so it could be difficultly to me to change the message.

Copy link
Contributor

Choose a reason for hiding this comment

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

The current master branch is for 2.0 so it is safe to change it. The message should be something like:

bad DVC file name '{}'. DVC files should be named '{PIPELINE_FILE}' or have a '.dvc' suffix (e.g. '{}.dvc')

Copy link
Contributor

@pmrowla pmrowla left a comment

Choose a reason for hiding this comment

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

Internally, outputs and dependencies apply to both pipeline (dvc.yaml) outputs and data files (.dvc) so there's a few places we should still be using the generic "DVC file"

dvc/output/base.py Outdated Show resolved Hide resolved
dvc/output/base.py Outdated Show resolved Hide resolved
dvc/dependency/base.py Outdated Show resolved Hide resolved
@efiop efiop added the awaiting response we are waiting for your reply, please respond! :) label Jan 28, 2021
@pmrowla
Copy link
Contributor

pmrowla commented Feb 4, 2021

hey @DiPaolo, any updates on this?

@DiPaolo
Copy link
Contributor Author

DiPaolo commented Feb 4, 2021

@pmrowla I've pushed an update.

@pmrowla pmrowla merged commit e7686f9 into iterative:master Feb 5, 2021
@pmrowla
Copy link
Contributor

pmrowla commented Feb 5, 2021

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response we are waiting for your reply, please respond! :)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

term: don't use "DVC-file" or "stage file" anymore, use ".dvc file"
3 participants