Adds the possibility to override values while generating the extension.json as described by issue #4320 #4861
Conversation
| @@ -59,6 +64,9 @@ | |||
| @Parameter(property = "bomVersion", defaultValue = "${project.version}") | |||
| private String bomVersion; | |||
|
|
|||
| @Parameter(property = "overridesFile", defaultValue = "${project.basedir}/src/main/resources/extensions-overrides.json") | |||
There was a problem hiding this comment.
this seems fishy - users can ovveride the name of resources folder - isn't there a more maven agnostic way to look these up ?
There was a problem hiding this comment.
${project.build.outputDirectory} the resources will end up being there, also possibly filtered.
There was a problem hiding this comment.
Using ${project.build.outputDirectory} doesn't seem to work. There's no output directory basically (there's just target, no target/classes)
There was a problem hiding this comment.
Before I was using ${project.resources[0].resource} but that doesn't work. The expression is probably too complex.
There was a problem hiding this comment.
Also, why is this fishy? All paths are basically overridable, right? I mean is there some way this could be abused?
There was a problem hiding this comment.
ah this the defaultvalue...oh well then i don't care at this stage - maybe post 1.0 if ever ;)
| } | ||
| } catch (IOException e) { | ||
| throw new IOException("Failed to parse " + descriptor, e); | ||
| } | ||
| } | ||
|
|
||
| private String extensionId(JsonObject extObject) { | ||
| return extObject.getString("groupId", "") + "." + extObject.getString("artifactId"); |
There was a problem hiding this comment.
this gives a NPE if you forget to put groupid/artifactid in extension entry. need to handle that better.
|
Left a few minor comments - if fixed lets merge it. One issue is though that using this does not result in categories being in the final document: |
| } | ||
| } catch (IOException e) { | ||
| throw new IOException("Failed to parse " + descriptor, e); | ||
| } | ||
| } | ||
|
|
||
| private String extensionId(JsonObject extObject) { | ||
| return extObject.getString("groupId", "") + "." + extObject.getString("artifactId"); |
There was a problem hiding this comment.
Why not keep the traditional : as a separator? It'll work either way but it'l be more intuitive and easier to read in case we need to display those if it is :.
@maxandersen where is that defined? I don't see anything about a categories section at the toplevel of that document being discussed in the issue? |
quintesse
force-pushed
the
patch_extensions_json
branch
from
0f9571c
to
0e6f1f1
Compare
because there aren't any (yet). in this case they are just added during this patching. |
|
I expected that just as I added a new "status" property inside extension I could add new "categoreies" property at the root. |
|
@maxandersen well that's very easy to do if you want that :-) |
|
@maxandersen well actually it's not that trivial, now that I think about it. So are you sure you want this? It's not the work (probably 15mins) but it's more about adding features that might need to be maintained etc |
|
We need someway to add categories. If not using this patch approach then we can do a separate file and bulk add it in. |
|
rebased against master and now merged. |
Issue #4320