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

Framework: add schema for current user #3356

Merged
merged 1 commit into from
Feb 17, 2016
Merged

Conversation

gwwar
Copy link
Contributor

@gwwar gwwar commented Feb 17, 2016

This adds a schema for current-user to avoid data shape changing errors, as described in #3101 Also tidies up a test to use sinon rather than mockery.

Testing Instructions

  • Start Calypso with:
    ENABLE_FEATURES=persist-redux make run
  • In the console set localStorage.debug to calypso:state
  • Refresh the page a few times and navigate to a few sections.
  • The app behaves normally and loads the persisted state/current-user from IndexedDB storage

cc @rralian @aduth @artpi @mtias

@gwwar gwwar added Framework [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 17, 2016
@gwwar gwwar self-assigned this Feb 17, 2016
@gwwar gwwar added this to the Calypso Core: Offline 4 milestone Feb 17, 2016
export default {
type: [ 'integer', 'null' ],
minimum: 0
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Arguing the smallest of points, but I wonder why ESLint doesn't identify this as needing a semi-colon? In my mind it should I'd think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

;) Looks complicated: eslint/eslint#2194 (comment)

@aduth
Copy link
Contributor

aduth commented Feb 17, 2016

Looks and works well for me 👍

On an unrelated note, do you have a recommended reference for JSON Schema aside from the website? The specs are... well, specs 😄

I do this tool handy for validating from a schema: http://jsonschemalint.com/

@aduth aduth added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 17, 2016
@gwwar
Copy link
Contributor Author

gwwar commented Feb 17, 2016

👍 @aduth thanks for reviewing!

On an unrelated note, do you have a recommended reference for JSON Schema aside from the website?

I've been a bit of a magpie and have been looking at a lot of misc docs. If I find a particularly good one I'll be sure to share it.

It does look like there's some nice generators that take in sample JSON and return a generated schema. This might be useful if we have a particularly large object.

@gwwar gwwar force-pushed the add/persist-current-user branch from 440586b to 78590ee Compare February 17, 2016 19:04
gwwar added a commit that referenced this pull request Feb 17, 2016
@gwwar gwwar merged commit 9aa6109 into master Feb 17, 2016
@gwwar gwwar deleted the add/persist-current-user branch February 17, 2016 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants