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

feat: add CookieAccessInfo.All sentinel #25

Merged
merged 1 commit into from
Jul 12, 2016

Conversation

keithamus
Copy link
Contributor

@keithamus keithamus commented Jun 13, 2016

This allows for fetching cookies with any access info, useful for tests.

Fixes #24. Refs chaijs/chai-http#43

Sorry this has taken a while to get done @bmeck

@keithamus
Copy link
Contributor Author

@bmeck any update on this?

@@ -12,6 +12,7 @@
}
return new CookieAccessInfo(domain, path, secure, script);
}
CookieAccessInfo.All = Object.freeze({});
Copy link
Owner

Choose a reason for hiding this comment

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

Object.freeze(Object.create(null)) if you would please, if you want a unique value, that is not a Symbol, create from null.

@bmeck
Copy link
Owner

bmeck commented Jul 12, 2016

@keithamus change to use Object.create(null) and LGTM

This allows for fetching cookies with any access info, useful for tests.

Fixes bmeck#24
@keithamus keithamus force-pushed the fix-24-add-cookieaccessinfo-all branch from 577c626 to a0a4cbd Compare July 12, 2016 19:56
@keithamus
Copy link
Contributor Author

@bmeck done and done 😄

@bmeck bmeck merged commit c9c3a0c into bmeck:master Jul 12, 2016
@bmeck
Copy link
Owner

bmeck commented Jul 12, 2016

merged, will publish to npm after a couple meetings

@keithamus keithamus deleted the fix-24-add-cookieaccessinfo-all branch July 12, 2016 19:59
@bmeck
Copy link
Owner

bmeck commented Jul 13, 2016

Published using f7c331b

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