The Wayback Machine - https://web.archive.org/web/20201218223048/https://github.com/expressjs/session/pull/635
Skip to content

/ session

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

Jump to bottom

Add promise support for sessionStore functions #635

Open
wants to merge 12 commits into
base: master
from
Open

Add promise support for sessionStore functions #635

wants to merge 12 commits into from

Conversation

@brandonmanke
Copy link

@brandonmanke brandonmanke commented Feb 24, 2019 •
edited

Potential solution to #607.

I can add some more tests to improve coverage if necessary.
@brandonmanke brandonmanke force-pushed the brandonmanke:session-store-promise branch from 6e29589 to ca343bd Feb 24, 2019
@brandonmanke
add: promise support for sessionStore save
7aaf1c0
@brandonmanke
add: promise support for sessionStore reload
f735620
@brandonmanke
add: promise support for sessionStore destroy
878f4ad
@brandonmanke
add: promise support for sessionStore regenerate
Loading status checks…
44cc8d1
@brandonmanke brandonmanke force-pushed the brandonmanke:session-store-promise branch from ca343bd to 44cc8d1 Feb 24, 2019
@dougwilson dougwilson force-pushed the brandonmanke:session-store-promise branch from 44cc8d1 to 63f0b88 Feb 24, 2019
@brandonmanke @dougwilson
add: promise support for sessionStore save
8081d4d
@brandonmanke @dougwilson
add: promise support for sessionStore reload
3d100d9
@brandonmanke @dougwilson
add: promise support for sessionStore destroy
55b218b
@brandonmanke @dougwilson
add: promise support for sessionStore regenerate
Loading status checks…
c687d5a
@dougwilson dougwilson force-pushed the brandonmanke:session-store-promise branch from 63f0b88 to c687d5a Feb 24, 2019
dougwilson added 2 commits Feb 26, 2019
@dougwilson
fixup! add: promise support for sessionStore destroy
Loading status checks…
28897aa
@dougwilson
fixup! add: promise support for sessionStore reload
Loading status checks…
cfd6abe
@brandonmanke
Copy link
Author

@brandonmanke brandonmanke commented Feb 27, 2019

@dougwilson I can add the should error without callback test for .reload() if that is ok? Also how would you recommend explicitly testing for errors with and without callbacks? Since the tests themselves will be almost identical.
@dougwilson
Copy link
Member

@dougwilson dougwilson commented Feb 27, 2019

Hey, so I added a couple more conditions on Monday to further bump the coverage to help out 👍Feel free to add some new ones as well; I just pushed up without the rebase on Monday just in case you were working on some as well.

I took a look at the coveralls config and it's set only to care about line coverage. So I wouldn't really worry about the reject branches to get it landed. If you want to add them anyway as an exercise, I would just copy-paste the tests at this time.
brandonmanke added 2 commits Mar 1, 2019
@brandonmanke
Merge branch 'session-store-promise' of https://github.com/brandonman…
89ff3d3 
…ke/session into session-store-promise
@brandonmanke
fix: redundant error checking in wrapmethods
Loading status checks…
23b0882
@brandonmanke
Copy link
Author

@brandonmanke brandonmanke commented Mar 2, 2019

How would you recommend testing https://coveralls.io/builds/21919265/source?filename=index.js#L385?
@whatl3y whatl3y mentioned this pull request Mar 22, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pr
2 participants
@brandonmanke @dougwilson
You can’t perform that action at this time.