Skip to content
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

Andre Kullmann make the profiling service configurable via java prope… #5027

Merged
merged 3 commits into from Feb 18, 2018

Conversation

Projects
None yet
2 participants
@areman
Copy link
Contributor

commented Feb 2, 2018

Currently the profiling service can only configured as program argument. With this pull request it will be possible to configure it as java property, too. This feature is importent to start the profiling if you don't run the jruby executable. e.g. you use jruby in a war archive or as script engine in a standard java application.

@kares

This comment has been minimized.

Copy link
Member

commented Feb 3, 2018

good, but those new TODOs of yours aren't needed. the runtime is essential, setter makes no sense - since its expected to stay the same...

@areman

This comment has been minimized.

Copy link
Contributor Author

commented Feb 3, 2018

Dear @kares,
yes the setter isn't needed. In my opinion it's not good design pattern to publish an interface which describes the methods to implement but implicit a special constructor is required. I'm not sure if the runtime performance will be a issue. Currently the constructor is called over reflection.

@kares

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

@areman thanks for the clarification and for the design pattern hints.
not sure if you'd like to get this PR merged or start a discussion with those TODOs.
if you'd like this in, please remove all of the redundant noise, otherwise should address those TODOs.

Andre Kullmann added some commits Feb 18, 2018

Andre Kullmann Andre Kullmann
@areman

This comment has been minimized.

Copy link
Contributor Author

commented Feb 18, 2018

Dear @kares,
yes I'll would like to see the request merged.
I've removed the TODO hints from the "productive" code.

@kares kares merged commit b22e2ce into jruby:master Feb 18, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@kares kares added this to the JRuby 9.2.0.0 milestone Feb 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.