Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Use immutable collections for user options #20

Merged
merged 8 commits into from Jun 26, 2014

Conversation

seanf
Copy link
Contributor

@seanf seanf commented May 19, 2014

Cleaner (I hope) solution for #17

@seanf seanf mentioned this pull request May 19, 2014
@buildhive
Copy link

Zanata » zanata-client #187 SUCCESS
This pull request looks good
(what's this?)

@buildhive
Copy link

Zanata » zanata-client #188 SUCCESS
This pull request looks good
(what's this?)

@Benaan
Copy link

Benaan commented May 19, 2014

I'm getting the follow error when pushing translations. It tries to add source files to the includes list, which happens to be immutable.

zanata-cli -B -e push -n -s . -t .

[ERROR] Execution failed:
java.lang.UnsupportedOperationException
at com.google.common.collect.ImmutableCollection.add(ImmutableCollection.java:92)
at org.zanata.client.commands.push.AbstractCommonPushStrategy.getSrcFiles(AbstractCommonPushStrategy.java:58)
at org.zanata.client.commands.push.AbstractPushStrategy.getSrcFiles(AbstractPushStrategy.java:101)
at org.zanata.client.commands.push.PropertiesStrategy.findDocNames(PropertiesStrategy.java:80)
at org.zanata.client.commands.push.PushCommand.pushCurrentModule(PushCommand.java:280)
at org.zanata.client.commands.push.PushCommand.run(PushCommand.java:203)
at org.zanata.client.commands.ConfigurableCommand.runWithActions(ConfigurableCommand.java:94)
at org.zanata.client.commands.ArgsUtil.runCommand(ArgsUtil.java:48)
at org.zanata.client.ZanataClient.processArgs(ZanataClient.java:168)
at org.zanata.client.ZanataClient.main(ZanataClient.java:93)

@seanf
Copy link
Contributor Author

seanf commented May 19, 2014

Thanks, Gertjan. Looking at the code again, the immutable rabbit hole is
deeper than I thought! I'll have to look into it some more.

On Mon, May 19, 2014 at 5:56 PM, Gertjan notifications@github.com wrote:

I'm getting the follow error when pushing translations. It tries to add
source files to the includes list, which happens to be immutable.

zanata-cli -B -e push -n -s . -t .

[ERROR] Execution failed:
java.lang.UnsupportedOperationException
at
com.google.common.collect.ImmutableCollection.add(ImmutableCollection.java:92)
at
org.zanata.client.commands.push.AbstractCommonPushStrategy.getSrcFiles(AbstractCommonPushStrategy.java:58)
at
org.zanata.client.commands.push.AbstractPushStrategy.getSrcFiles(AbstractPushStrategy.java:101)
at
org.zanata.client.commands.push.PropertiesStrategy.findDocNames(PropertiesStrategy.java:80)
at
org.zanata.client.commands.push.PushCommand.pushCurrentModule(PushCommand.java:280)
at org.zanata.client.commands.push.PushCommand.run(PushCommand.java:203)
at
org.zanata.client.commands.ConfigurableCommand.runWithActions(ConfigurableCommand.java:94)
at org.zanata.client.commands.ArgsUtil.runCommand(ArgsUtil.java:48)
at org.zanata.client.ZanataClient.processArgs(ZanataClient.java:168)
at org.zanata.client.ZanataClient.main(ZanataClient.java:93)


Reply to this email directly or view it on GitHubhttps://github.com//pull/20#issuecomment-43474787
.

@seanf
Copy link
Contributor Author

seanf commented May 20, 2014

@Benaan I've had another go.

@Benaan
Copy link

Benaan commented May 20, 2014

@seanf Thanks for your effort. I've tested it and it works. I've compared the logs of a push command with my original fix and yours and they are the exactly the same. Pull commands also seem to work.

@carlosmunoz carlosmunoz self-assigned this May 20, 2014
@carlosmunoz
Copy link
Member

@seanf I've taken the liberty of reviewing this code. Looks good.
👍

@seanf
Copy link
Contributor Author

seanf commented May 21, 2014

Thanks @carlosmunoz.

It's amazing how I can turn a one line fix into 123 lines. The test coverage for the client isn't the best, so I'm a little worried that we might try to add to these immutable collections somewhere (there were a few of these already).

Perhaps we should use @carlosmunoz 's quick and dirty fix (no offence!) in the release branch, and let this version stew in integration/master for a while.

@seanf
Copy link
Contributor Author

seanf commented May 21, 2014

See #22 for the quick fix.

@definite
Copy link
Member

definite added a commit that referenced this pull request Jun 26, 2014
@definite definite merged commit 4fddff2 into integration/master Jun 26, 2014
@definite definite deleted the immutable-collections branch June 26, 2014 00:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants