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

Plist settings over UserDefaults #117

Merged
merged 1 commit into from
Jan 10, 2018

Conversation

JoeSSS
Copy link
Contributor

@JoeSSS JoeSSS commented Dec 29, 2017

Respect plist settings over UserDefaults

@JoeSSS JoeSSS requested review from q231950 and BasThomas December 29, 2017 13:29
@@ -36,10 +36,14 @@ class SettingsViewController: NSViewController {
(linkField4, linkDescriptionField4)
]

if let cucumberProfile = applicationStateHandler.cucumberProfile {
if let cucumberProfile = plistOperations.readValues(forKey: Constants.Keys.cucumberProfileInfo).first {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about keeping this logic in the ApplicationStateHandler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also wanted to do so, but then decided to do not, as if some users already saved this value in their settings, the next run will explicitly give them a warning asking to add the profile with available options.

Copy link
Contributor

@BasThomas BasThomas Jan 10, 2018

Choose a reason for hiding this comment

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

I don't get that? What I mean is that you can just do applicationStateHandler.cucumberProfile in which internally (so in the StateHandler) it does readValues instead of reading from UserDefaults.

Copy link
Contributor

Choose a reason for hiding this comment

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

#128 has been created to tackle this 👍

@JoeSSS JoeSSS merged commit e548865 into save_all_settings_in_plist Jan 10, 2018
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.

2 participants