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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Feb 18, 2018
Merged

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

merged 3 commits into from
Feb 18, 2018
+14 −3

Conversation

areman
Copy link
Contributor

@areman areman 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
Copy link
Member

kares 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
Copy link
Contributor Author

areman 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
Copy link
Member

kares 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.

@areman
Copy link
Contributor Author

areman 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
@kares kares added this to the JRuby 9.2.0.0 milestone Feb 18, 2018
enebo added a commit that referenced this pull request Oct 21, 2019
@enebo
Supplemental Fix for #5027. This uses rubyName() which will appropria…
914e38c 
…tely

encode the exception class if it happens to have non-default encoding
unfriendly characters.  As a result of that change we can then use the
str() signature which allows us to eliminate Java String concatenation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
JRuby 9.2.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@areman @kares @andrekullmann