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

Allow set and save to be chained #15

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

aitorres
Copy link

@aitorres aitorres commented Dec 15, 2021

Resolves #11

What I did

  • Modified the settings base class to return self on both set() and save(), allowing these two methods to be chained as an alternative to setsave()
  • Added a new test case to ensure that set().save() works as expected
  • Also, I added a new test case to the base settings tests to make the same check for setsave()
  • Fixed setsave() calls in YAMLSettings and TOMLSettings, as well as in JSONSettings, since they weren't passing the option and value arguments to the super call. This was uncovered after adding the test for setsave mentioned above.

At this point I haven't removed the setsave method (in case it should remain for compatibility reasons), and I haven't (yet) made any changes to the README, changelog or docs, but I could do if needed!

How to test

I added two new tests to test_settingsbase, all tests should pass with pytest

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.

Allow .set(), and .save() to be chainable.
1 participant