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

New setting in store provider denoting whether to use the internal cache#29

Merged
ntotten merged 4 commits intoauth0:masterfrom
tocr:feature/useCacheSetting
Mar 30, 2016
Merged

New setting in store provider denoting whether to use the internal cache#29
ntotten merged 4 commits intoauth0:masterfrom
tocr:feature/useCacheSetting

Conversation

@tocr
Copy link
Copy Markdown

@tocr tocr commented May 21, 2015

  • new setting added into the store provider
  • caching adjusted in the Internal store to reflect the useCache setting
  • test cases added

Comment thread dist/angular-storage.js Outdated
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.

add a space after the if

@mgonto
Copy link
Copy Markdown
Contributor

mgonto commented May 21, 2015

Thanks for the PR :).

Only comment is that since you've added useCache before the delimited, that means this is a breaking change :D which I don't want. Could you move it to after the delimeter.

If we were to do a breaking change, I'd prefer to change the parameters to be options so that order doesn't matter, but for next version :D.

Thanks!

@tocr
Copy link
Copy Markdown
Author

tocr commented May 25, 2015

Thanks for your comments. I made changes to preserve the API of the library.

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.

use angular.isDefined

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.

Actually, just set this.useCache = !!useCache. If undefined or null, it'll be false :).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oh, not again cast-to-bool operator :)
Anyway, I don't think it would work. !! defaults null/undefined to false. I need to default it to true.

Or, I could revert the semantics and rename it to 'dontUseCache' Then the !! would work I think

@mgonto
Copy link
Copy Markdown
Contributor

mgonto commented May 26, 2015

A few more comments, but it looks really good.

Thanks!

@mgonto
Copy link
Copy Markdown
Contributor

mgonto commented Jul 22, 2015

Can you rebase with amster and we'll merge?

Thanks!

@tocr
Copy link
Copy Markdown
Author

tocr commented Aug 12, 2015

I made a merge from upstream. I am sorry for the delay.

@acorbel
Copy link
Copy Markdown

acorbel commented Dec 10, 2015

Hi,

Any update for this PR?

@pranshunz
Copy link
Copy Markdown

Whats the status of this PR?

@ntotten ntotten merged commit 6cc8d7b into auth0:master Mar 30, 2016
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.

6 participants