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

#1618 Custom message when cache and remote are in sync #4694

Merged
merged 3 commits into from
Oct 19, 2020
Merged

#1618 Custom message when cache and remote are in sync #4694

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b736a66
#1618 Custom message when cache and remote are in sync
tenitski Oct 11, 2020
256dcce
Output the correct remote name during dvc status
tenitski Oct 11, 2020
2bbfe81
Simplify logic
tenitski Oct 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions dvc/command/status.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class CmdDataStatus(CmdDataBase):
STATUS_LEN = 20
STATUS_INDENT = "\t"
UP_TO_DATE_MSG = "Data and pipelines are up to date."
IN_SYNC_MSG = "Cache and remote '{remote}' are in sync."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There could be a better phrased message, I'm open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, Cache and remote are in sync sounds a bit like cache and remote have the same list of cache files, which is actually not true, because we care about the cache for the local workspace or branches/tags (--all-tags, --all-branches). But this is still a good start 👍

Copy link
Contributor Author

@tenitski tenitski Oct 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I used this paragraph from dvc status documentation as a guide for what is happening and how it should be phrased:

For new and deleted data, the cache is different from remote storage. Bringing the two into sync requires dvc pull or dvc push. For the typical process to update the workspace, see Sharing Data And Model Files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So if you don't need to do anything to bring them into sync I thought we can call the state as "are in sync" :)

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'm keen to work out the best message cause this PR is all about making the message more accurate. Having an OK message kind of defeats the purpose...

Copy link
Contributor

@efiop efiop Oct 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@tenitski Thanks for the attention to details! 🙏

@jorgeorpinel could you take a look, please?

Copy link
Contributor

@jorgeorpinel jorgeorpinel Oct 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Minor: based on the elif order below wouldn't it be better to put this new constant before UP_TO_DATE_MSG?

As for the text itself, it seems good but mentioning the cache seems too low-level. Maybe "This repo is in sync with the remote." ? Also, may be no need to specify which remote since the user should know that already... But I guess for when you have a default remote, to make sure you're aware of which one was checked, appending the remote name at the end is not a bad idea.

EMPTY_PROJECT_MSG = (
"There are no data or pipelines tracked in this project yet.\n"
"See {link} to get started!"
Expand Down Expand Up @@ -68,6 +69,12 @@ def run(self):
self._show(st, indent)
elif not self.repo.stages:
logger.info(self.EMPTY_PROJECT_MSG)
elif self.args.cloud or self.args.remote:
logger.info(
self.IN_SYNC_MSG.format(
remote=self.repo.config["core"].get("remote")
efiop marked this conversation as resolved.
Show resolved Hide resolved
)
)
else:
logger.info(self.UP_TO_DATE_MSG)

Expand Down