search

rjz / supertest-session

Persistent sessions for supertest
Other
89 stars 21 forks source link

Document cookie access & upgrade supertest #36

Closed ctavan closed 5 years ago

ctavan commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

Would require a major version bump.

ctavan commented 5 years ago

@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 commented 5 years ago

Coverage Status

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

ctavan commented 5 years ago

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 commented 5 years ago

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

rjz commented 5 years ago

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