Skip to content

Push#127

Merged
jdavid merged 63 commits intodevelopfrom
push
Aug 14, 2018
Merged

Push#127
jdavid merged 63 commits intodevelopfrom
push

Conversation

@jdavid
Copy link
Contributor

@jdavid jdavid commented Aug 9, 2018

Push

jdavid added 30 commits April 12, 2018 10:43
Rsh1a New push.admin.publish
Will be reused in future PRs
RSH1b3 New push.admin.device_registrations.save
Rsh1b1 New push.admin.device_registrations.get
Rename to get_channel_name, as it makes more sense imho. Make it a
classmethod, so it can be called from setUpClass. Use assert in a
few places (we should do that everywhere, now we use pytest).
Sometimes, when the sandbox is under heavy load, we get timeout errors.
Be easy on the sandbox by not running the tests in parallel. The speed
we get by running the tests in parallel does not compensate the
troubling random errors.

Also, don't need to use tox in travis.

Also, run flake8 only over the project files.
RSH1b2 New push.admin.device_registrations.list
RHS1b4 New push.admin.device_registrations.remove
RHS1b5 New push.admin.device_registrations.remove_where
The call is async, wait a little bit longer before giving up.
We moved from unittest to pytest a while back, but we're still using
unittest idioms. While they work we should use pytest idioms instead.

This may help debugging/fixing the random errors we see in Travis.
@mattheworiordan
Copy link
Member

@jdavid I assume this incorporates all the PRs that we have already approved? So are we just approving this being merged into develop and being released? If so, we may want to tag 1.1. @paddybyers WDYT?

@jdavid
Copy link
Contributor Author

jdavid commented Aug 9, 2018

Yes, all the PRs already approved.

@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 9033560 on push into f97945c on develop.

@funkyboy
Copy link
Contributor

funkyboy commented Aug 9, 2018

I’d tag after it’s been merged. Much like we do for other releases.

@paddybyers
Copy link
Member

So are we just approving this being merged into develop and being released?

Yes, pls merge. Thanks.

If so, we may want to tag 1.1. @paddybyers WDYT?

It needs to advertise the 1.1 protocol version in the request param/header, because the realtime system implements either the old or new spec for push authentication based on that. So I guess we could also release this as a 1.1 pre-release, or just as 1.1.0 but with a note in the README about features not yet implemented from the 1.1 spec. I've had a look though what applies to REST, and it's mainly the JWT stuff (with various other auth-related clarifications) plus idempotent publishing. I think we would want both of those before we properly label it a 1.1 release.

@mattheworiordan
Copy link
Member

I think we would want both of those before we properly label it a 1.1 release.

I thought we had agreed we were happy to release a lib with a version claim so long as it states which features it does not yet support clearly? ably/docs#482. So 1.1.0 for this release is my preference.

And then hopefully we can add JWT tests & Idempotent after for a patch bump.

@jdavid jdavid merged commit 6c28ebc into develop Aug 14, 2018
@jdavid jdavid deleted the push branch August 14, 2018 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants