Skip to content

Various fixes from previous PRs #413

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

specialtactics
Copy link
Contributor

@specialtactics specialtactics commented Jul 15, 2024

Context

These are our previous fixes from some previous PRs, I think they were overriding the composer.json package name so were not merged.

What has been done

Fixes various issues which remain in the master branch.

Notes

We've been maintaining a fork of this package with these fixes (at covergenius/php-vcr), however it's great to see that this package is once again maintained, we are just contributing back what has not yet been addressed since our fork.

@higidi
Copy link
Contributor

higidi commented Oct 29, 2024

Hi @specialtactics

thx 4 your contributions (and backports).

I'll merge them back as soon as possible.

Can you first fix the pipeline please?

@higidi
Copy link
Contributor

higidi commented Oct 29, 2024

Stop @specialtactics

there are several other issues regarding this issue #363 #241 #227 , also pull requests, but this implementation changes the public api, so i'll have to create a major release. I'll refactor this, be backward compatible and release 1.9.0 as soon as possible. So you must not do anything!

@specialtactics
Copy link
Contributor Author

Hey @higidi it would be really great to merge this.

I understand what you mean about the backwards compatible API change, I think the reason that was the case was argument optionality, but I changed it now so it should be backwards compatible, meaning no new version needed. I synced with other upstream changes from master.

At the moment there are some soap tests failing, however I'm not sure why or if it's relevant to these changes. I tried to test locally, but weirdly I get errors about inclusion of libraries (note I don't have issues with other repos).

Would you mind having a look, and can you confirm if you do a fresh checkout of this repo locally, can you run tests without problems?

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