Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Empty pref file is an empty object, not an error.#7263

Merged
redmunds merged 2 commits intomasterfrom
dangoor/7220-empty-prefs
Mar 21, 2014
Merged

Empty pref file is an empty object, not an error.#7263
redmunds merged 2 commits intomasterfrom
dangoor/7220-empty-prefs

Conversation

@dangoor
Copy link
Copy Markdown
Contributor

@dangoor dangoor commented Mar 20, 2014

This is a fix for #7220 (errors caused by a prefs file being empty).

This also fixes #7229 which was duped to #7220.

@redmunds redmunds self-assigned this Mar 20, 2014
Comment thread test/spec/PreferencesBase-test.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think you even need the fail() block since waitsForDone() already verifies that it didn't fail.

I reviewed the rest of this btw and it looks good... so @redmunds / @dangoor if you don't think the above extra bit of code is a big deal feel free to merge now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I think the waitsForDone() is enough, so the fail() should be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll remove that.

@redmunds
Copy link
Copy Markdown
Contributor

Done with review.

@dangoor
Copy link
Copy Markdown
Contributor Author

dangoor commented Mar 21, 2014

Fixed my silliness.

@redmunds
Copy link
Copy Markdown
Contributor

Merging.

redmunds added a commit that referenced this pull request Mar 21, 2014
Empty pref file is an empty object, not an error.
@redmunds redmunds merged commit e08abaf into master Mar 21, 2014
@redmunds redmunds deleted the dangoor/7220-empty-prefs branch March 21, 2014 17:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error reading preferences Sprint 37

3 participants