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

fix: lint error from new phpseclib #515

Closed
wants to merge 1 commit into from
Closed

Conversation

bshaffer
Copy link
Contributor

@bshaffer bshaffer commented Jan 2, 2024

Fixes a linting error caused in the latest version (3.0.35) of PHPSeclib due to the AsymmetricKey::toString function having its return type changed from string to string|array

@bshaffer bshaffer requested a review from a team as a code owner January 2, 2024 22:23
@bshaffer bshaffer added the release blocking Required feature/issue must be fixed prior to next release. label Jan 2, 2024
ajupazhamayil
ajupazhamayil previously approved these changes Jan 3, 2024
@vishwarajanand
Copy link
Contributor

This is already being covered here: #513

@ajupazhamayil
Copy link

Removing approval here as I am not sure which PR will be merged.. both look the same to me.. Thank you!

@ajupazhamayil ajupazhamayil requested review from ajupazhamayil and removed request for ajupazhamayil January 3, 2024 08:41
@ajupazhamayil ajupazhamayil dismissed their stale review January 3, 2024 08:45

Not sure this PR will be merged or the other one pointed out in this PR.

@bshaffer
Copy link
Contributor Author

bshaffer commented Jan 3, 2024

@vishwarajanand

This is already being covered here: #513

@ajupazhamayil

both look the same to me

That PR is addressing two issues in one, and also the fix is not the same as I've implemented (there's an exception being thrown for $key->getPublicKey(); for no reason that I can tell).

If you'd like to split the fix out into a separate PR and remove the other exception being thrown, I'm happy to approve it and merge it, but we shouldn't put two different things in a single PR.

@vishwarajanand
Copy link
Contributor

We can close this.. I am splitting this PR: #513

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release blocking Required feature/issue must be fixed prior to next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants