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

add CacheClearer #839

Merged
merged 2 commits into from Mar 7, 2021
Merged

add CacheClearer #839

merged 2 commits into from Mar 7, 2021

Conversation

gam6itko
Copy link
Contributor

@gam6itko gam6itko commented Mar 5, 2021

Q A
Bug fix? yes
New feature? yes
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #834
License MIT

Works only with schmittjoh/metadata#97

Copy link
Collaborator

@goetas goetas left a comment

Choose a reason for hiding this comment

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

i've released your changes in the metadata package as 2.5.0 (see https://github.com/schmittjoh/metadata/releases/tag/2.5.0)

can you add it as requirement in the composer.json?

Cache/CacheClearer.php Outdated Show resolved Hide resolved
Cache/CacheClearer.php Outdated Show resolved Hide resolved
@gam6itko gam6itko closed this Mar 7, 2021
@goetas goetas reopened this Mar 7, 2021
@goetas goetas merged commit 4102cb7 into schmittjoh:master Mar 7, 2021
@goetas
Copy link
Collaborator

goetas commented Mar 7, 2021

thank you!

@gam6itko
Copy link
Contributor Author

gam6itko commented Mar 7, 2021

@goetas Is it acceptable that it failed the test with --prefer-lowest?

@goetas
Copy link
Collaborator

goetas commented Mar 7, 2021

Hmm, I did not notice that... Ideally it should be fixed... 😔

@gam6itko
Copy link
Contributor Author

gam6itko commented Mar 7, 2021

@goetas problem in CacheClearerTest. I can rewrite it according to --prefer-lowest

@gam6itko gam6itko deleted the cache_clear branch March 7, 2021 20:28
@@ -233,6 +233,11 @@
<tag name="jms_serializer.deserialization_visitor" format="xml" />
</service>

<service id="jms_serializer.cache.cache_clearer" class="JMS\SerializerBundle\Cache\CacheClearer" public="false">
<argument type="service" id="jms_serializer.metadata.cache"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is missing on-invalid="null entry, as disabling cache (with cache: none) is broken now.

The constructor should be updated to accept null then. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix it ASAP

Copy link
Contributor

Choose a reason for hiding this comment

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

I have already opened a PR #850., would be nice if you could review it. 👍

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.

None yet

3 participants