Skip to content

Conversation

@skshetry
Copy link
Collaborator

@skshetry skshetry commented Jan 19, 2022

By splitting dvc.cli into a module, we'll be able to share cli-related
utilities, split parser-related stuffs into smaller parts, improve
reusability and improve API for command extensions in the future.

This PR moves main entrypoint from dvc.main.main to dvc.cli.main, but the old one is still kept for backward compatibility, but that should be considered deprecated after this PR is merged and set for removal in 3.0.

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

@skshetry skshetry added refactoring Factoring and re-factoring A: cli Related to the CLI labels Jan 19, 2022
@skshetry skshetry requested review from efiop and pmrowla January 19, 2022 06:35
@skshetry skshetry self-assigned this Jan 19, 2022
@skshetry skshetry requested a review from a team as a code owner January 19, 2022 06:35
By splitting dvc.cli into a module, we'll be able to share cli-related
utilities, split parser-related stuffs into smaller parts, improve
reusability and improve API for command extensions in the future.
# so won't be reused by any other subsequent run anyway.
clean_repos()
# TODO: remove in 3.0, kept for backward compatibility
from .cli import main # noqa: F401, pylint: disable=unused-import
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Keeping for backward compatibility

Copy link
Contributor

Choose a reason for hiding this comment

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

@skshetry Could you elaborate, please? We never promised this as an API, so looks like we can get rid of it right away.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, but I have been suggesting it to the users in Discord as a way to use dvc as a Python API in a stable manner. Could remove though.

@efiop
Copy link
Contributor

efiop commented Jan 19, 2022

@skshetry Should dvc.command be part of dvc.cli?

@skshetry skshetry closed this Jan 19, 2022
@skshetry skshetry reopened this Jan 19, 2022
@skshetry
Copy link
Collaborator Author

skshetry commented Jan 19, 2022

@skshetry Should dvc.command be part of dvc.cli?

I don't think so. dvc.cli is about entry point and cli-parsing utilities. #7259 and #7272 has evolved my understanding about dvc.command as being plugin. (not to add, it adds one level of nesting).

I have a few minor refactors after this:

  1. Move dvc.command.base to dvc.cli.command.
  2. Rename dvc.command to dvc.commands.
  3. Change dvc.exceptions.DvcParser to not inherit from DvcException as it's strictly dvc.cli only thing. It'll be moved as dvc.cli.DvcParserError.

@efiop efiop merged commit f5baf99 into treeverse:main Jan 19, 2022
@skshetry skshetry deleted the cli-refactor branch January 19, 2022 10:36
@skshetry skshetry restored the cli-refactor branch April 27, 2022 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: cli Related to the CLI refactoring Factoring and re-factoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants