-
Notifications
You must be signed in to change notification settings - Fork 73
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
Updating JCommander to 1.78 #615
Conversation
@@ -345,7 +345,6 @@ public void importXcodeXliff() throws Exception { | |||
|
|||
getL10nJCommander().run("push", "-r", repository.getName(), | |||
"-s", getInputResourcesTestDir("source").getAbsolutePath(), | |||
"-t", getInputResourcesTestDir("translations").getAbsolutePath(), |
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.
The Push command doesn't have any -t
parameter
@@ -206,7 +206,8 @@ public void createJCommanderForRun() { | |||
logger.debug("Create JCommander instance"); | |||
jCommander = new JCommander(); | |||
|
|||
jCommander.setAcceptUnknownOptions(true); | |||
// TODO: Cant enable this until https://github.com/cbeust/jcommander/issues/377 is fixed | |||
// jCommander.setAcceptUnknownOptions(true); |
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.
I think that was used to pass extra configuration to spring ... not sure if it is actually used today
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.
@jeeyi Is this used on your side?
@@ -119,7 +119,7 @@ public Long answer(InvocationOnMock invocation) throws Throwable { | |||
|
|||
localizeDropFiles(dropRepository.findById(dropId).orElse(null)); | |||
|
|||
l10nJCommander.run(new String[]{"drop-import", "-r", repository.getName(), "--number-drop-fetched", "1000"}); |
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.
😨
b0d9400
to
87ad622
Compare
Fixes #612
There's a known issue with
jCommander.setAcceptUnknownOptions(true)
, described here. Disabling that option and updating tests that were using unknown parameters helped to get a green build.