Remove deprecated cloning pattern config which isn't really useful anymore (revive this November 2024) #41621
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
holly-cummins
changed the title
This comment has been minimized.
This comment has been minimized.
holly-cummins
force-pushed
the
remove-cloning-config
branch
from
adad725 to
3e8de31
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Comment from July: We should merge this a couple of releases after #41623, so perhaps November 2024.
#40601 dropped support for the class-clone-pattern configuration, but didn’t remove the documentation. The configuration wasn’t really needed anymore, since #40601 cloned almost everything.
My amendment of #40601 to work around the “do not clone everything” problem clones less, and does reintroduce have the support for configuring the cloning. However, I think even in this scenario, the config is not useful. The config was introduced as part #17306, to make Pact work, but I've run the Pact tests against a local quarkus build and Pact works fine without the config.
I’ve searched through public codebases, and I do not believe this configuration is widely used. https://github.com/search?q=quarkus.test.class-clone-pattern%3D&type=code shows only 6 usages of
quarkus.test.class-clone-patternin configuration files, all in commented out boilerplate. Searching for the camel case version,CLASS_CLONE_PATTERN=gave no instances: https://github.com/search?q=CLASS_CLONE_PATTERN%3D&type=codeIn my searches, I added the equals sign so filter out config documentation and forks of the quarkus repo, which do reference the property name.
The default value of the configuration is
java.*, so removing the config does mean we no longer attempt to clonejava.*classes. I don’t see any benefit in doing that cloning, but be aware this (sort of) is a functional change.