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 tests #32

Merged
merged 3 commits into from
Feb 5, 2023
Merged

Fix tests #32

merged 3 commits into from
Feb 5, 2023

Conversation

WalterProof
Copy link

@WalterProof WalterProof commented Feb 5, 2023

Fixes 2 errors,

  • one about missing doctrine annotations service:
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: The service "doctrine.orm.default_annotation_metadata_driver" has a dependency on a non-existent service "doctrine.orm.metadata.annotation_reader".

I believe migrating phpdoc annotation to php annotation is ok to fix this.

  • and the other about doctrine bundle shard option:
doctrine:database:create [-c|--connection [CONNECTION]] [--if-not-exists]

.

  The "--shard" option does not exist.

so I just removed it

see doctrine/DoctrineBundle#1467

But maybe the doctrine/shards dev dependency and shard related tests should also be removed

@theofidry
Copy link
Owner

Thanks a lot for the PR and for looking into this!

see doctrine/DoctrineBundle#1467
But maybe the doctrine/shards dev dependency and shard related tests should also be removed

I was not aware, indeed then it makes sense to remove it, but if possible I would prefer this to be done in a separate PR.

Regarding the PR, to me it looks good, but I would prefer if the refactoring to move to annotations could be extracted into a dedicated 🙏

@WalterProof
Copy link
Author

WalterProof commented Feb 5, 2023

Oh yes, I restored the original LoadDataFixturesCommandIntegrationTest, and the tests are still green, the The "--shard" option does not exist. is just polluting the PHPUnit output.
So, this one is the one for moving to PHP annotation. And I'll make another one for the shard removal, if it's ok like that.
Should I stash into one commit?
I may also separate the cs fix if you want

@theofidry theofidry merged commit 33f78ea into theofidry:master Feb 5, 2023
@theofidry
Copy link
Owner

Thank you @der-alter!

@WalterProof WalterProof deleted the fix_tests branch February 6, 2023 08:49
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