Make sure quarkus-extension.yaml can be parsed using CatalogMapperHelper #21661
Conversation
quarkus-bot
bot
added
area/devtools
|
I'll need to add a test making sure |
| final ExtensionCatalog.Mutable mutableCatalog = extensionCatalog instanceof ExtensionCatalog.Mutable | ||
| ? (ExtensionCatalog.Mutable) extensionCatalog | ||
| : extensionCatalog.mutable(); | ||
| extensions.forEach(mutableCatalog::addExtension); | ||
| this.extensionCatalog = extensionCatalog; |
There was a problem hiding this comment.
Should we be using a built catalog in the event we had to change it / add extensions?
...cts/tools/registry-client/src/main/java/io/quarkus/registry/catalog/CatalogMapperHelper.java
Show resolved
Hide resolved
| } | ||
| extensions.addAll(extensionCatalog.getExtensions()); | ||
| ((ExtensionCatalog.Mutable) extensionCatalog).setExtensions(extensions); | ||
| final ExtensionCatalog.Mutable mutableCatalog = extensionCatalog instanceof ExtensionCatalog.Mutable |
There was a problem hiding this comment.
Can we make the mutable() return this in ExtensionCatalog.Mutable implementations? It feels wrong to check for an instanceof first
There was a problem hiding this comment.
mutable() acts as a copy constructor, "create a new thing based on this current thing". If mutable() returns this, that ability goes away.
Note that I'll be hitting all of this code again (in a second pass), trying to hide more things.. so it may be that this check goes away for other reasons...
There was a problem hiding this comment.
Perhaps there should also be clone()?
There was a problem hiding this comment.
And mutable() would return this for a mutable instance.
There was a problem hiding this comment.
Do you want to do that with this fix? Or defer to next round.
There was a problem hiding this comment.
I can but this there is also no rush if you like to think about it more. Let me know what you prefer.
There was a problem hiding this comment.
Let me do it in the next rev.. so I can make sure there aren't any knock-on effects.
aloubyansky
force-pushed
the
extension-yaml-parsing
branch
from
107861b to
a6c5cd6
Compare
aloubyansky
force-pushed
the
extension-yaml-parsing
branch
from
a6c5cd6 to
c34e734
Compare
|
@ebullient I added a test |
This fixes a regression in codestart testing when
quarkus-extension.yamlare parsed after the extension catalog API and JSON binding refactoring change.