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

Remove 99designs/http-signatures dependency for stability #63

Merged
merged 7 commits into from
Feb 26, 2018

Conversation

hannesvdvreken
Copy link
Contributor

@hannesvdvreken hannesvdvreken commented Feb 26, 2018

Removed

  • Dependency on dated version of 99designs/http-signatures
  • HMAC class, inlined the functionality into Signer class.
  • public Signer::urlSafeB64encode method. Was not supposed to be used as part of public interface of this package.
  • signature_middleware_factory global method in helpers.php file.

Changed

  • Batcher constructor (was not supposed to be used outside of $client->batcher())

Added

  • public method Signer::signRequest which handles Request signing for Batcher.

Done this because of GetStream/stream-laravel#65. Now we can install next version (2.6) together with Laravel 5.6 and Symfony 4.0. (Symfony 4, dependency of Laravel 5.6 was not compatible with previous version of 99designs/http-signatures)

This way we don't need to tag new major version (v2.5.2 -> v3.0.0) because of upgrading an underlying dependency.

@hannesvdvreken hannesvdvreken merged commit 50b9961 into master Feb 26, 2018
@hannesvdvreken hannesvdvreken deleted the fix/99designs branch February 26, 2018 12:03
Copy link
Contributor

@dwightgunning dwightgunning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say too much for functionality / semantics. Should we sit together and install / run / test it on my machine?

Visual coding style check passes for me.

@hannesvdvreken
Copy link
Contributor Author

@dwightgunning All good 👍 did all the checks locally to ensure it does install nicely with Laravel 5.6 and Symfony 4.0, which was the goal. And all integration tests succeed as well, and did some extra manual tests too.

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

2 participants