crowdin / crowdin-cli

A command-line client for the Crowdin API
https://crowdin.github.io/crowdin-cli
MIT License
250 stars 94 forks source link

fix: throw error if no translations were found #824

Closed yevheniyJ closed 4 months ago

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 65.90%. Comparing base (f742d84) to head (946d6fe).

Files Patch % Lines
...m/crowdin/cli/commands/actions/DownloadAction.java 50.00% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #824 +/- ## ========================================= Coverage 65.90% 65.90% Complexity 1514 1514 ========================================= Files 223 223 Lines 6160 6160 Branches 932 932 ========================================= Hits 4059 4059 Misses 1562 1562 Partials 539 539 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sbrandwoo commented 4 months ago

Please can you consider reverting this change. We run crowdin-sync via the github action and now our jobs are failing with the following:

❌ Couldn't find any file to download. Since you are using the 'Skip untranslated files' option, please make sure you have fully translated files

There's no error here, everything is working as expected, but now the process is failing.

ben-wharton commented 4 months ago

As @sbrandwoo says, we run this workflow daily on >100 repos in our infra, and this caused many engineers to wonder why their builds appear to be failing (whereas really their translations just aren't ready yet).

andrii-bodnar commented 4 months ago

@sbrandwoo @ben-wharton Sorry for the inconvenience.

If the skip untranslated files option is enabled, we can probably still exit with code 0 as before. It looks like that would be pretty expected in this case.

@yevheniyJ, what do you think?

yevheniyJ commented 4 months ago

Agree, sorry for inconvenience. PR with fix is prepared https://github.com/crowdin/crowdin-cli/pull/829 CC @andrii-bodnar

andrii-bodnar commented 4 months ago

@sbrandwoo @ben-wharton the new versions are already available:

https://github.com/crowdin/crowdin-cli/releases/tag/4.1.1 https://github.com/crowdin/github-action/releases/tag/v2.1.1