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

chore: allow zircote/swagger-php v5 #2420

Merged
merged 3 commits into from
Jan 17, 2025
Merged

Conversation

bobvandevijver
Copy link
Contributor

@bobvandevijver bobvandevijver commented Jan 15, 2025

Description

zircote/swagger-php v5 has been released, and seems to only require minimal updates. See https://github.com/zircote/swagger-php/blob/master/docs/guide/migrating-to-v5.md.

I did need to migrate the current processor implementation to the one that is using the new processor pipeline introduced with 4.10.0, which is why I bumped the minimum constraint. In order to not mess up dependency injection, I was required to wrap the Generator class in order to allow adding additional processors using the existing compiler pass.

What type of PR is this? (check all applicable)

  • Bug Fix
  • Feature
  • Refactor
  • Deprecation
  • Breaking Change
  • Documentation Update
  • CI

Checklist

  • I have made corresponding changes to the documentation (docs/)
  • I have made corresponding changes to the changelog (CHANGELOG.md)

@bobvandevijver bobvandevijver changed the title Allow zircote/swagger-php v5 chore: Allow zircote/swagger-php v5 Jan 15, 2025
@bobvandevijver
Copy link
Contributor Author

bobvandevijver commented Jan 15, 2025

Well, that went well 😄 I'm looking into it

Ready for review! 😄

@bobvandevijver bobvandevijver changed the title chore: Allow zircote/swagger-php v5 chore: allow zircote/swagger-php v5 Jan 15, 2025
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.56%. Comparing base (e44364d) to head (6d24589).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2420      +/-   ##
==========================================
- Coverage   89.56%   89.56%   -0.01%     
==========================================
  Files          78       79       +1     
  Lines        2914     2912       -2     
==========================================
- Hits         2610     2608       -2     
  Misses        304      304              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bobvandevijver bobvandevijver force-pushed the patch-1 branch 2 times, most recently from 1bc3b7b to 701ab46 Compare January 15, 2025 10:16
@bobvandevijver
Copy link
Contributor Author

bobvandevijver commented Jan 15, 2025

Looking at the code coverage report: I do not think that the before option has ever worked at all, because the tags are name indexed array, and thus not parsed correctly. I've solved that with #2421, and there will be a merge conflict with this change fixed!.

Because of the string before support I needed to upgrade the minimum to 4.11.1, because otherwise more code from zircote would needed to be integrated here.

Copy link
Collaborator

@DjordyKoert DjordyKoert left a comment

Choose a reason for hiding this comment

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

Thank you! ❤️

@DjordyKoert DjordyKoert merged commit a2497e1 into nelmio:master Jan 17, 2025
23 checks passed
@bobvandevijver bobvandevijver deleted the patch-1 branch January 17, 2025 12:22
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.

2 participants