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

Document cookie access & upgrade supertest #36

Merged
merged 4 commits into from
Mar 28, 2019
Merged

Conversation

ctavan
Copy link
Collaborator

@ctavan ctavan commented Mar 25, 2019

Would be great to get an upgraded version of supertest-session.

I was also considering removing supertest as a dependency and instead declaring it as a peer-dependency so that users of supertest-session can decide on their own, which version of supertest to use.

What do you think about that, @rjz ?

@rjz
Copy link
Owner

rjz commented Mar 26, 2019

@ctavan, I think adding peerDependencies (at this point, most of this project's dependencies probably count) makes sense. Happy to merge this as-is, or merge that change, too.

In any event, thanks for these! Overdue and much appreciated.

@ctavan
Copy link
Collaborator Author

ctavan commented Mar 26, 2019

Will move to peerDependencies since this should be more future proof.

Would require a major version bump.

@ctavan ctavan force-pushed the upgrade-supertest branch 3 times, most recently from a03d05b to 1ccd46b Compare March 27, 2019 22:19
@ctavan
Copy link
Collaborator Author

ctavan commented Mar 27, 2019

@rjz I've moved supertest into a peerDependency and modernized the test setup a bit (see commit messages for details).

I'd leave bumping the version up to you.

We could of course use the opportunity of a major version bump to introduce the change documented in #27 as a new default behavior. If you want I can update that other PR to introduce that change before we do the major version bump.

WDYT?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 95.833% when pulling a03d05b on ctavan:upgrade-supertest into 7b9cbc1 on rjz:master.

Also add an installation note to the readme as this now requires to
manually install supertest alongside supertest-session.

The former dependencies
* cookiejar
* methods
now come as transitive dependencies through supertest.
Use nyc instead of jasmine-node and only test in reasonably modern
node.js versions in TravisCI.
@ctavan
Copy link
Collaborator Author

ctavan commented Mar 27, 2019

Oh and is there a particular reason you run coveralls only on master? (I removed the master restriction for one CI run, hence the report above)

@rjz
Copy link
Owner

rjz commented Mar 28, 2019

No good reason here (and thanks for all of the long-overdue updates!)

@rjz rjz merged commit 5a264d5 into rjz:master Mar 28, 2019
@rjz
Copy link
Owner

rjz commented Mar 28, 2019

Alright, these are merged and released in v4.0.0. Thanks again, @ctavan!

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.

None yet

3 participants