Skip to content

repro: update status message for dvc-added files#4317

Merged
efiop merged 2 commits into
treeverse:masterfrom
sarthakforwet:repro_update
Aug 6, 2020
Merged

repro: update status message for dvc-added files#4317
efiop merged 2 commits into
treeverse:masterfrom
sarthakforwet:repro_update

Conversation

@sarthakforwet
Copy link
Copy Markdown
Contributor

It is same PR as #4269.

@sarthakforwet

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

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

Thanks!

@efiop efiop merged commit 4759517 into treeverse:master Aug 6, 2020
@efiop
Copy link
Copy Markdown
Contributor

efiop commented Aug 6, 2020

For the record: all occurences of this message in the docs are for stages, so no need to adjust anything there.

Comment thread dvc/stage/__init__.py
if not (kwargs.get("force", False) or self.changed()):
logger.info("Stage '%s' didn't change, skipping", self.addressing)
if not isinstance(self, PipelineStage) and self.is_data_source:
logger.info("'%s' didn't change, skipping", self.addressing)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should not this be foo didn't change rather foo.dvc?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought the same @skshetry but changed my mind on the argument that as .dvc files maintain the updates in the original files hence mentioning foo.dvc rather than foo sounded logical to me.

Also .dvc files are kind of representatives of the original files so using them in the place of original ones should be good IMHO.

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.

3 participants