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

feat(API): Add 'update_translation_keys' for Uploads [TSI-2292] #578

Merged
merged 2 commits into from
Apr 29, 2024

Conversation

hahmed-dev
Copy link
Contributor

Documents the newly introduced option update_translation_keys for POST#Upload.

Related Issues:

@hahmed-dev hahmed-dev self-assigned this Apr 17, 2024
@hahmed-dev hahmed-dev requested a review from jablan April 17, 2024 13:04
paths/uploads/create.yaml Outdated Show resolved Hide resolved
@hahmed-dev hahmed-dev requested a review from jablan April 17, 2024 13:20
@@ -106,7 +107,7 @@ public void uploadCreateTest() throws ApiException, IOException, InterruptedExce
Boolean autotranslate = null;
Boolean markReviewed = null;
Boolean tagOnlyAffectedKeys = null;
Upload response = api.uploadCreate(projectId, xPhraseAppOTP, branch, file, fileFormat, localeId, tags, updateTranslations, updateDescriptions, convertEmoji, skipUploadTags, skipUnverification, fileEncoding, localeMapping, formatOptions, autotranslate, markReviewed, tagOnlyAffectedKeys);
Upload response = api.uploadCreate(projectId, xPhraseAppOTP, branch, file, fileFormat, localeId, tags, updateTranslations, updateTranslationKeys, updateDescriptions, convertEmoji, skipUploadTags, skipUnverification, fileEncoding, localeMapping, formatOptions, autotranslate, markReviewed, tagOnlyAffectedKeys);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's interesting that this was needed. I thought this parameter is not required? any idea why it's part of the signature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the newly built Java client, in fact other params that are marked as optional are all passed to the method. I guess all these arguments have to be passed to the method, we can set null for the ones that are optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because if I am not wrong, a Java method would expect all the options to be passed, its within the method where we check if the option is present, then assign it to the payload.
Screenshot 2024-04-17 at 15 34 55

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, technically that means that any such change should be marked as breaking for java lib :( but I guess we won't be doing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats a good point, should we maybe change feat to feat! in the commit message? To represent a potentially breaking change via major version bump?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. This change shouldn't be a breaking one, we added an optional field. However, seems to me that Java generator is not correct so it generates a breaking change for Java. I don't think we have a way right now to declare breaking changes only for a single platform.
So, I'd keep this a minor version bump as usual, but we should be aware of this Java generator quirk, and perhaps try to address it in the future (perhaps updating the Java templates to newer version would be enough, although even that is no easy job).

@hahmed-dev
Copy link
Contributor Author

Pending confirmation of the fix from customer. Once received, docs will be updated.

@infotexture
Copy link

@hahmed-dev How does this new parameter differ from update_translations? 🤔

📖 It doesn't seem to be mentioned yet in the CLI docs or sample configuration file.

@hahmed-dev
Copy link
Contributor Author

@infotexture thanks for pointing out, at the moment its only documented in API docs. We will have CLI relevant docs updated as well.

How does this new parameter differ from update_translations?

This option prevents changing any attribute of an existing key or creating a new one (if its missing) during upload, by default its true (existing behaviour). Whereas update_translations determines if or not to update the actual translation.

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.

3 participants