Fix issue #394 by making the addDebugLicense function protected instead of private#403
Conversation
|
converting to draft here because after a private deployment of the artifact the calls fail again |
Did you restart the Gradle daemon in the meantime, or was that failure still related to the 'old' private method trying to be invoked? |
I'm still checking, but I think the wrong code was deployed to the internal maven (with the private function). Still leaves me with the tests that don't test correctly. |
|
ready for review. The issue is fixed by making the function protected instead of private but to be honest I don't know why it works in the tests and not in a bigger project. |
|
Thanks for the contribution! I was doing a bit of digging on my own and it's an odd bug, caused by Groovy being very dynamic and requiring Gradle to invoke methods at runtime via reflection. It seems like depending on how the build runs, the call to Either way, your private-protected fix works, so I'm good to take it. I don't think the tests will be helpful since this is semi-non-deterministic. Go ahead and remove them and I'll approve the merge. It may take a few weeks for a release to get out the door though, and I have a few other issues that I want to look into to hopefully include. Incidentally, this wouldn't have happened if the plugin was written in Kotlin. I've been resisting the urge to convert it to Kotlin just for the sake of it, but this gives me some real reason to. |
|
Done. This breaks the PR down to one word 😆 |
|
I guess I cannot merge this PR since I don't have write access to the project. @timothyfroehlich You likely have to merge it yourself. |
|
Whoops, merged! I'm going to try to wrap up another cleanup change this week to include in the next release, which I should be able to make in the next week or two (it depends on another team's release schedule). |
First of all thank you to @rvandermeulen for finding the fix itself.
Currently the tests do NOT fail on the current main state (so effectively they do not reproduce the issue at hand), but I could validate that the change fixes builds in our project. I will dedicate some more time to the tests to properly reproduce the issue in them. @mreichelt and @rvandermeulen feeld free to participate in the conversation
fixes #394