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

OptionHandler's getMetaVariable fails when used with ResourceBundle #109

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sannies
Copy link

@sannies sannies commented Apr 7, 2015

the method checks if there is a localization for the metavar and assumes that ResourceBundle#getString(key) will return null when there is no key. But the getString method will throw a MissingResourceException instead. Therefore it is not possbile to use getString with L10N.

(sorry for re-format of the test class)

@buildhive
Copy link

Kohsuke Kawaguchi » args4j #168 UNSTABLE
Looks like there's a problem with this pull request
(what's this?)

@buildhive
Copy link

Kohsuke Kawaguchi » args4j #169 SUCCESS
This pull request looks good
(what's this?)

@buildhive
Copy link

Kohsuke Kawaguchi » args4j #172 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@buildhive
Copy link

Kohsuke Kawaguchi » args4j #173 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@kohsuke
Copy link
Owner

kohsuke commented Aug 16, 2015

Unfortunately, too many whitespace changes make it difficult to review this diff

@buildhive
Copy link

Kohsuke Kawaguchi » args4j #177 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@buildhive
Copy link

Kohsuke Kawaguchi » args4j #178 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@sannies
Copy link
Author

sannies commented Aug 17, 2015

Sorry for creating such a sloppy pull request in the first place. I tidied up the pull request - it should be very easy to review now.
Latest build is failing due to hiccup on jenkins.

@javaerb
Copy link
Contributor

javaerb commented Jan 30, 2016

can this be merged?

@sannies
Copy link
Author

sannies commented Jan 31, 2016

In my eyes it's ready to merge.

  • Test is there.
  • fixes an obvious problem as rb.getString(token) cannot be expected to return null according to JavaDoc (MissingResourceException - if no object for the given key can be found)
  • The last build problem was only a hiccup on jenkins

@sannies
Copy link
Author

sannies commented Mar 17, 2017

Hi,
it there anything I can do to get this in? I'm still running on my patched branch.
Best Regards,
Sebastian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants