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

Bugfix settings dict lookup #1369

Merged
merged 3 commits into from May 22, 2019

Conversation

@avanwinkle
Copy link
Collaborator

commented May 17, 2019

This PR fixes a bug in SettingsController that prevented programmatic access to the machine settings.

The settings are stored as machine.__dict__["_settings"] (with underscore) but the lookup item's check was against machine.__dict__["settings"] (without underscore). As a result, any attempt to access machine.settings["foo"] would throw.

This PR adds the underscore so the lookup is in the correct place and machine settings values are accessible.

@jabdoa2

This comment has been minimized.

Copy link
Collaborator

commented May 17, 2019

I guess we should also add a test to keep it working.

Anthony van Winkle and others added some commits May 19, 2019

Anthony van Winkle Anthony van Winkle
Merge branch 'dev' into machine-settings-bugfix
* dev:
  Util string_to_list with braces (#1361)
@avanwinkle

This comment has been minimized.

Copy link
Collaborator Author

commented May 20, 2019

Now with tests!

Also, noticed that I accidentally pushed this branch to the main repo instead of my fork. Whoops!

@jabdoa2

This comment has been minimized.

Copy link
Collaborator

commented May 22, 2019

Thanks for adding tests. PR looks good to merge for me!

@jabdoa2 jabdoa2 merged commit d66faac into dev May 22, 2019

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jabdoa2 jabdoa2 deleted the machine-settings-bugfix branch May 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.