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

halconfigurer: Overhaul of configurer, see below for details #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

richteer
Copy link
Contributor

@richteer richteer commented Oct 1, 2018

So this overhaul became a bit more of a mess than expected, but I'm liking so far how it is coming out.

I had two main objectives:

  1. Allow validation of preexisting config blobs
  2. Allow validation of individual config keys/items.

So far, both of these options probably should work, I will be updating this branch as I fix/tweak things.

Changelog (so far):

  • .configure() method of internal HalConfigurer objects now defines a list of option strings
  • External callers should now instantiate a HalConfigurer object, and either iterate through the options and call .ask(), or .validate() an option
  • .ask() now takes a function argument for how to receive the value
  • added a depends= optional parameter to options, since .configure() on HalConfigurer objects no longer is the code that prompts users, therefore breaking conditional prompts like in irc

NOTE: I am unable to test any of these changes outside of the main.py cli due to halibot-extra/irc#3, hooray

- .configure() method of internal HalConfigurer objects now defines a list of
    option strings
- External callers should now instantiate a HalConfigurer object, and either
    iterate through the options and call .ask(), or .validate() an option
- .ask() now takes a function argument for how to receive the value
@richteer richteer added the RFC label Oct 1, 2018
@richteer richteer requested a review from sjrct October 1, 2018 17:13
@richteer
Copy link
Contributor Author

richteer commented Oct 1, 2018

Oh yeah, Travis builds are gonna be hella broken. Will fix the tests in a later commit


if missing and not fill_default:
str = "Missing key" + ("s" if len(missing) > 1 else "") + ": " + ", ".join(missing)
raise KeyError(str)
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 you need a return ret here?

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.

2 participants