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: Php8.4 take 2 #839

Merged
merged 11 commits into from
Jan 10, 2025
Merged

fix: Php8.4 take 2 #839

merged 11 commits into from
Jan 10, 2025

Conversation

phpfui
Copy link
Contributor

@phpfui phpfui commented Jan 8, 2025

I pushed the fix to the old branch. The fix is removing the typed parameters from Twilio\Version::stream. Not sure why I changed this, as it was not typed in a doc block. But also not sure why the tests on the branch would not fail in the same way.

Once we are confident all the tests that should run pass, we should be able to merge again.

@phpfui
Copy link
Contributor Author

phpfui commented Jan 10, 2025

Let's get this merged for the next release. The PR was previously approved, I just fixed the errors from the tests. If there are any further issues, then I can fix. But that would also indicate there is a mismatch between the main branch tests and PR branches, which we should figure out.

@tiwarishubham635
Copy link
Contributor

Let's get this merged for the next release. The PR was previously approved, I just fixed the errors from the tests. If there are any further issues, then I can fix. But that would also indicate there is a mismatch between the main branch tests and PR branches, which we should figure out.

Thanks for pointing this out. I'll check this once. Let's try to merge it

Copy link
Contributor

@tiwarishubham635 tiwarishubham635 left a comment

Choose a reason for hiding this comment

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

LGTM

@tiwarishubham635 tiwarishubham635 merged commit 92f82cd into twilio:main Jan 10, 2025
19 checks passed
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.

3 participants