Add CLI option to append tags on push #23
Conversation
dbendilas
left a comment
dbendilas
left a comment
There was a problem hiding this comment.
I've added a couple of comments. Also I'm not sure what part of the code actually takes sends the tags to CDS. Will that be in another PR?
| // Append custom tags | ||
| if (tags != null && tags.length != 0) { | ||
| for (LocaleData.StringInfo stringInfo : sourceStringMap.values()) { | ||
| if (stringInfo.meta == null) { | ||
| stringInfo.meta = new LocaleData.StringInfo.Meta(); | ||
| } | ||
| stringInfo.meta.tags = tags; | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there a reason we're not always adding the Meta? It would make more sense to me to have it always and add the if just before setting stringInfo.meta.tags = tags;.
There was a problem hiding this comment.
At this moment in time, Meta only contains tags and tags can't be parsed by the XML converter (in the previous step). So yeah, we could always add it since it's always null.
But in the future, the Meta could also contain developer comments or other stuff (which are supported by the spec). So, why overwrite it since the XML Parser could have populated it?
| assertThat(parsedPostData.data.get("tx_test_key").meta.tags).asList().containsExactly("some_tag"); | ||
| assertThat(parsedPostData.data.get("tx_plural_test_key").meta.tags).asList().containsExactly("some_tag"); |
There was a problem hiding this comment.
Let's test with multiple tags, not just one at the time.
Petrakeas
force-pushed
the
petrakeas/append-tags-cli
branch
from
b23b4b2 to
266f8c7
Compare
LocaleData.StringInfo class has now a "Meta" subclass that contains "tags" in the form of a set according to the spec: https://github.com/transifex/transifex-delivery/#push-content Added appendTags() method for easier appending of tags. The following methods have been updated to take the "meta" field into account: equals(), toString() Added "--append-tags" option in CLI's MainClass. It is optional and can accept zero to multiple tags.
Petrakeas
force-pushed
the
petrakeas/append-tags-cli
branch
from
266f8c7 to
7f300d5
Compare
LocaleData.StringInfo class has now a "Meta" subclass
that contains "tags" in the form of a string array according to
the spec: https://github.com/transifex/transifex-delivery/#push-content
The following methods have been updated to take the "meta" field into account:
equals(), toString()
Added "--append-tags" option in CLI's MainClass. It is optional and can accept
zero to multiple tags. The tags are appended to the parsed strings.