Not setting configured labels on PRs

[torokati44]

These lines should probably use a /pulls/ URL, rather than /issues/...

github-action/entrypoint.sh

Line 161 in c953b17

ISSUE_URL="${REPO_URL}/issues/${PULL_REQUESTS_NUMBER}"

github-action/entrypoint.sh

Line 178 in c953b17

ASSIGNEES_URL="${REPO_URL}/issues/${PULL_REQUESTS_NUMBER}/assignees"

See:
https://github.com/ruffle-rs/ruffle/actions/runs/11021911424/job/30610062421#step:4:11
https://github.com/ruffle-rs/ruffle/actions/runs/11021911424/job/30610062421#step:4:576

Docs:
https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#update-a-pull-request

[andrii-bodnar]

@torokati44 thanks for reporting this! It's strange, I wonder how it worked before.

I just made a change in the fix-pulls branch (812dd3c)

Could you please try it by referencing the branch instead of the version?

uses: crowdin/github-action@fix-pulls

[torokati44]

Maybe it never worked...? 😐🫥
Thank you for the quick fix!
I think actually testing it on our end would have a bit too much overhead, especially since we're in the process of moving away from this action anyway... 😶
( ruffle-rs/ruffle#18028 )

[andrii-bodnar]

since we're in the process of moving away from this action anyway

Could you please provide more details on this?

If you need to make some changes to the translation files, you are probably looking for post-export processing on the Crowdin side instead of doing it after the translations download.

[torokati44]

Would you mind summarizing, @kjarosh?

[kjarosh]

We want to localize the following files:

https://github.com/ruffle-rs/ruffle/blob/b59ee2f91bf0c98c3fb6e200f8878c925bc5c84e/desktop/packages/linux/rs.ruffle.Ruffle.metainfo.xml

https://github.com/ruffle-rs/ruffle/blob/b59ee2f91bf0c98c3fb6e200f8878c925bc5c84e/desktop/packages/linux/rs.ruffle.Ruffle.desktop

Crowdin does not support them, so the plan is to:

1. convert them to .pot and feed it to Crowdin on upload,
2. download .pos and merge them into the final file on download.

Conversion tools include xgettext, msgfmt, and itstool.

[andrii-bodnar]

@kjarosh thanks for the details!

I just did a quick test and it seems that the provided XML file imports well without any modifications or convertations. You can additionally specify XPath translatable_elements to better configure what is translatable (Configuration File).

The *.desktop file looks very similar to ini. If you change the extension, it imports very well as an ini file format.

So, you can add an extra step to your workflow where you rename all the *.desktop files to *.ini, then do the sync via Crowdin action (with PR creation disabled). After that, rename ini files back and create a pull request. I hope this should work.

[torokati44]

I believe the problem is that both the metainfo and desktop files have specific semantics of how they are supposed to store the localized texts (all in a single file) - it's not as simple as with other files, where it's just multiple instances of the same basic file (one per language).
Hence these file formats (semantically speaking, not just about how they "look") would need tailor-made support from Crowdin.
But correct me if I'm wrong.

[andrii-bodnar]

Do you mean that these files are multilingual?

[kjarosh]

@torokati44 is right, Crowdin indeed can translate those files, but we'll get an instance for each locale, which is not how metainfo and desktop files support localization.

The desktop file uses [<locale>] suffix for keys (for translatable elements), and metainfo uses inline xml:lang attributes (for translatable elements).

If we have to merge those files anyway, it's just easier to convert them to .pot/.po using standard methods.

---

Do you mean that these files are multilingual?

That's right, you can consult the documentation here:

• metainfo: https://freedesktop.org/software/appstream/docs/chap-Metadata.html
• desktop: https://specifications.freedesktop.org/desktop-entry-spec/latest/localized-keys.html

[andrii-bodnar]

Thanks for the details! This sounds very reasonable to me now.

Earlier we had a similar discussion to allow running some pre-commit scripts, but there is no obvious solution on how to do it right (#181)

You can still synchronize the source files and translations with this action, but disable the automatic commit and PR creation.

[kjarosh]

You can still synchronize the source files and translations with this action, but disable the automatic commit and PR creation.

Yes, that's the plan. I think @torokati44 meant that we're moving away from this action regarding committing & creating PRs.

[torokati44]

Indeed, pardon my imprecision.

[torokati44]

Actually, it's entirely possible that the /issues/ URL does get redirected to the correct one, based on the number (if it actually belongs to a PR); and it doesn't work for us because the GITHUB_TOKEN we supply has no permission to manage labels... But in that case an error message about this would have been nice...

[ratchanonsuttawas]

These lines should probably use a /pulls/ URL, rather than /issues/...

github-action/entrypoint.sh

Line 161 in c953b17

ISSUE_URL="${REPO_URL}/issues/${PULL_REQUESTS_NUMBER}"

github-action/entrypoint.sh

Line 178 in c953b17

ASSIGNEES_URL="${REPO_URL}/issues/${PULL_REQUESTS_NUMBER}/assignees"

See:
https://github.com/ruffle-rs/ruffle/actions/runs/11021911424/job/30610062421#step:4:11
https://github.com/ruffle-rs/ruffle/actions/runs/11021911424/job/30610062421#step:4:576

Docs:
https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#update-a-pull-request