-
Notifications
You must be signed in to change notification settings - Fork 133
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
A few fixes to support phpseclib 2 #72
Conversation
@foreachlt thanks, I'll give it a look this week. I'm a little bit worried and we should triple check because probably there are some cases not covered by tests (we already tried once). |
@cirpo thank you for taking a look at this 👍 |
we're really looking towards this merge/fix (too) because without this, we can't use google api client in version 2 because it requires phpseclib2 which conflicts with your library. Thanks for fixing it @foreachlt |
@foreachlt could you please rebase this PR with the latest master? We've just released version 6.0.4. |
@cirpo will do 😉 |
@foreachlt if you could use php-cs-fixer with |
@cirpo it should be all done 👍 |
@cirpo ohh, will apply php fixer stuff too, just a sec |
@foreachlt @tristanbes I will try to make it by the end of this week, thanks a lot guys! |
@cirpo php-cs-fixer hasn't found any violations in |
@foreachlt 👍 |
@cirpo what prevents this PR to be merged and tag a release right now if all the tests pass? I'm curious. |
@tristanbes last time the tests where passing but someone experienced some issues. We need to check the coverage. I can't do it right now as I'm a bit busy and I would avoid the same mistake we did last time. |
ok thanks for your answser :) |
@foreachlt I created a new branch phpseclib-2 https://github.com/namshi/jose/tree/phpseclib-2. Could you please open the PR against that branch? I will be easy for everyone to test it, wdyt? |
@foreachlt @tristanbes everything seems working fine on our side. @foreachlt you get phpseclib ~2.0 the 2.0-dev version, what if we specify the 2.0.0? what are your thoughts about it? |
@cirpo sure, I agree with you. I didn't realize it would get |
👍 for |
@foreachlt if you want you can do the modification :) |
@cirpo changed the version. Rebased, tests still pass locally. All good to I think 😉 |
yalla, lets merge! |
A few fixes to support phpseclib 2
@foreachlt @tristanbes enjoy the 6.1.0 release thanks a lot folks! |
@cirpo Wondered if this dependency could be upgraded? I feel like I fixed the broken bits and the tests pass 😉
Thanks.