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

sopel.cli.utils #1472

Merged
merged 8 commits into from Feb 28, 2019
Merged

sopel.cli.utils #1472

merged 8 commits into from Feb 28, 2019

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Feb 6, 2019

Related to #1471:

  • create a sopel.cli.utils modules,
  • functions from sopel.cli.utils are available from sopel.cli as shortcuts,
  • find, enumerate, and load configurations functions are now in sopel.cli.utils,
  • sopel.run_script now uses sopel.cli.utils for these functions,
  • existing tests are moved around into test/cli/test_cli_utils.py for consistency,

and there is no new feature, only internal changes: everything is backward compatible.

From this PR, it is possible to change how settings are loaded (which files, from where, etc.), and there is one single place to load a Sopel's configuration.

@Exirel
Copy link
Contributor Author

Exirel commented Feb 7, 2019

Oh, so in Py2, the subparser action is mandatory... interesting!

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Once again, the majority of line notes are related to docstrings.

There are relatively more larger questions in this review, though. That's why it's a "Comment" instead of "Request changes"—the changes I might request aren't defined yet.

sopel/config/__init__.py Outdated Show resolved Hide resolved
sopel/cli/utils.py Outdated Show resolved Hide resolved
sopel/cli/utils.py Outdated Show resolved Hide resolved
sopel/cli/utils.py Outdated Show resolved Hide resolved
sopel/cli/utils.py Outdated Show resolved Hide resolved
sopel/cli/utils.py Show resolved Hide resolved
sopel/cli/utils.py Outdated Show resolved Hide resolved
sopel/cli/utils.py Outdated Show resolved Hide resolved
sopel/run_script.py Show resolved Hide resolved
sopel/run_script.py Show resolved Hide resolved
@dgw dgw added this to the 7.0.0 milestone Feb 8, 2019
@dgw dgw added the Feature label Feb 8, 2019
@Exirel Exirel force-pushed the sopel-cli-utils branch 2 times, most recently from f2770ac to 4f7e500 Compare February 9, 2019 06:31
@Exirel
Copy link
Contributor Author

Exirel commented Feb 9, 2019

@dgw should be good for one last review.

@dgw dgw self-requested a review February 9, 2019 13:28
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

:shipit:

@dgw dgw merged commit 460f9dd into sopel-irc:master Feb 28, 2019
@Exirel Exirel deleted the sopel-cli-utils branch March 21, 2019 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants