Conversation
| protected abstract ProtobufTestAdapter createAdapter(Path workdir); | ||
|
|
||
| @Test | ||
| public void shouldInvokeMockSubprocessForExternalPlugin() throws Exception { |
There was a problem hiding this comment.
For reviewers, this test shows the exposed "enabling" functionality
|
Think this is a great start! When I have some time again I'll see if I can hook it into the tooling on my side and see how that goes! Thanks for putting the time into getting this working. |
core-test/src/main/java/io/roastedroot/protobuf4j/test/AbstractCliTest.java
Show resolved
Hide resolved
|
@ascopes have you had the chance to try to integrate those changes into your pipeline, does it helps? |
|
@andreaTP I haven't just yet unfortunately, things have been pretty manic recently. Need to spend some time trying to work out the best way of exposing this such that it still remains totally agnostic. Apologies for the delay. It is still on my radar 🙂 |
|
Thanks for getting back, that's fine, it's enough to hear is on your radar, take your time! |
|
@andreaTP regarding getting a build to try this on, am I able to just fudge the WASM binaries in from this branch into my local Maven repository, or does it require a build from scratch to work? I might have a little time today to fiddle around with some stuff. |
|
You can use the binaries committed on this branch 👍 |
|
Just having a look at this, are we setting the Main-Class Manifest attribute anywhere for the core-v4 package? Thinking we need to override the Maven JAR Plugin in the core-v3 and core-v4 projects to inject the Main-Class attribute into the manifest so the JAR can be executable without users needing to explicitly find out and specify the main class FQDN. (Aware the classpath of the dependency tree is also missing from above but I have logic already in place to be able to compute the appropriate values for this already so that side of this should be fine). |
not yet, it should be easy to do so though, either in the core package or in a dependent "CLI" module. |
|
@andreaTP not blocked per sé as luckily on my side I've already got the ability to inject the entrypoint. Was mostly mentioning in case you wanted to include it on this PR |
|
Good to hear! |
2 participants
This is a step forward to, eventually, enable #30 , I finally took the time to look into it.
I know this is not "exactly" in the direction you envision @ascopes , still it should enable the use case.
Please, @ascopes , verify those changes, and we can move from there.