-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Premain-Class attribute #297
Conversation
@nickcaballero Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
@nickcaballero Thank you for signing the Contributor License Agreement! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR ! and sorry to only look into it now.
So, I have several comments (some are about problems, while other are about cosmetics)
First, can you please rebase your PR on top of the latest master ?
Also, can you please describe your new feature somewhere in the documentation.
For example, in the Installation
section of the quick_start.md
, you could add some kind of the following:
If you can't change the application code, you can also activate BlockHound using the
-javaagent:<path to BlockHound agent jar>
jvm option, something like:java -javaagent:BlockHound/agent/build/libs/agent.jar -jar my-application.jar
Now, please have a look at the following review, hope you will have time to work on this before we do the next release, let me know
thanks !
I just rebased this PR on top of latest master. |
@pderop I've updated the PR with your suggestions! Went with the |
thanks a lot for updating the PR 😃, One last small thing, could you just add a note somewhere in the documentation, please check #297 (review) ? thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the PR !
+1 from me (I'll update the doc about using the -javaagent in another new PR).
@reactor/core-team , @reactor/netty-team , can someone of you review all this ? thanks.
@nickcaballero, thanks for this PR ! I'll change the parameter names of the premain method in another PR, let's merge this one for the moment. |
This PR adds
premain
method and manifest attribute, supporting the installation of the agent via-javaagent=
JVM option.