Skip to content

Improve synchronization process when no pull request is needed - #20896

Merged
mikehardy merged 1 commit into
ankidroid:mainfrom
DoomsCoder:sync_translation
May 29, 2026
Merged

Improve synchronization process when no pull request is needed#20896
mikehardy merged 1 commit into
ankidroid:mainfrom
DoomsCoder:sync_translation

Conversation

@DoomsCoder

Copy link
Copy Markdown
Contributor

Note

Syntax research for the GitHub API was assisted by Gemini-3-flash

Purpose / Description

I fixed the translation sync workflow so it doesn't error out when there aren't any string to update

Fixes

Approach

I used github-script to check for staged changes and manually trigger a neutral check result via the GitHub API if the tree is clean

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@david-allison

Copy link
Copy Markdown
Member

Could this be tested?

@DoomsCoder

DoomsCoder commented Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

Yes, I can test it running workflow on my fork. I'll temporarily use GITHUB_TOKEN to bypass the missing organisation token secrets

@DoomsCoder
DoomsCoder force-pushed the sync_translation branch 5 times, most recently from 60f466c to a224b38 Compare April 29, 2026 21:26
@DoomsCoder

Copy link
Copy Markdown
Contributor Author
image

I have sucessfully tested the workflow. You can see the results.

@DoomsCoder
DoomsCoder force-pushed the sync_translation branch 2 times, most recently from 112e301 to aacb745 Compare April 29, 2026 21:42
@david-allison

david-allison commented Apr 30, 2026

Copy link
Copy Markdown
Member

Can I confirm it looks like this on the status preview in the actions tab (screenshot from Google images; 6 years ago on a different screen)

I wouldn't have expected a green tick like I see in your screenshots

181086584-e7553285-4ce6-4cf5-b220-e309f202e236

EDIT: I had a quick chat with Claude and it looks like this may not be possible from a workflow perspective; only with the checks API:

If it's not possible, let's move to the issue, discuss another fix (GITHUB_STEP_SUMMARY or similar)

@DoomsCoder

DoomsCoder commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author
image

@david-allison I'm sorry for confusion. I really took screenshot from my own fork Action tab and I definitely didn't take from Google images. I used Checks API to create a neutral status, which is why its appeared that in testing

@DoomsCoder

DoomsCoder commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

EDIT: I had a quick chat with Claude and it looks like this may not be possible from a workflow perspective; only with the checks API:

If it's not possible, let's move to the issue, discuss another fix (GITHUB_STEP_SUMMARY or similar)

Should i stick with Checks API for neutral status(current approach) or switch to GITHUB_STEP_SUMMARY for simple approach?

@david-allison

Copy link
Copy Markdown
Member

I know the screenshot was yours, I checked and I saw the effort you put in, it's truly appreciated.

I strongly suspect it won't be possible, so let's go with another approach:

  • Consider an obviously named step, then skipping the remainder of the work if there's no changes
    • A developer can quickly see from the
  • Consider options to output text to GitHub. I personally use the step summary, but I haven't researched what's available

Whatever you do will be better than what we have now, and it's truly appreciated!

@DoomsCoder
DoomsCoder force-pushed the sync_translation branch 2 times, most recently from d625a03 to a0986e1 Compare May 1, 2026 14:48
@DoomsCoder

Copy link
Copy Markdown
Contributor Author
image

@david-allison I have tested the step summary approach on my fork. You can check it out in the screenshot

@DoomsCoder

DoomsCoder commented May 1, 2026

Copy link
Copy Markdown
Contributor Author

Windows test failure seems unrelated to my code changes

@Alok-Silswal

Alok-Silswal commented May 3, 2026

Copy link
Copy Markdown
Contributor

Windows test failure seems unrelated to my code changes

Have you tested this?

If no, then try running unit tests without your code changes.

Saying this because I highly doubt that earlier commits would not have been accepted without all unit tests passing.

@david-allison

Copy link
Copy Markdown
Member

Unit tests were flaky.

Comment thread .github/workflows/sync_translations.yml Outdated
Comment thread .github/workflows/sync_translations.yml Outdated
Comment thread .github/workflows/sync_translations.yml
@DoomsCoder
DoomsCoder force-pushed the sync_translation branch 2 times, most recently from 0aa46ca to e4a0f5e Compare May 10, 2026 15:12
@DoomsCoder

Copy link
Copy Markdown
Contributor Author

@david-allison I updated the code. See if you like it

@DoomsCoder
DoomsCoder requested a review from david-allison May 10, 2026 15:15

@david-allison david-allison left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is excellent, thank you!

Comment thread .github/workflows/sync_translations.yml Outdated
@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels May 10, 2026

@mikehardy mikehardy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! In general I like the idea / the purpose of the PR - definitely needs a different take on the conflict resolution, and an idea on compacting the implementation tho

Comment thread .github/workflows/sync_translations.yml Outdated
Comment thread .github/workflows/sync_translations.yml Outdated
@DoomsCoder
DoomsCoder requested a review from mikehardy May 15, 2026 18:38
@DoomsCoder

Copy link
Copy Markdown
Contributor Author

@mikehardy I've updated the code. Please see if this is the compact implementation you were looking for

@DoomsCoder

Copy link
Copy Markdown
Contributor Author

@mikehardy Could you please review the updated code when you have time ?

@mikehardy mikehardy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

seems good to go now, thank you!

@mikehardy mikehardy added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge Has Conflicts labels May 29, 2026
@mikehardy
mikehardy added this pull request to the merge queue May 29, 2026
Merged via the queue into ankidroid:main with commit 5a70ba6 May 29, 2026
19 checks passed
@github-actions github-actions Bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label May 29, 2026
@github-actions github-actions Bot added this to the 2.25 release milestone May 29, 2026
@DoomsCoder
DoomsCoder deleted the sync_translation branch June 3, 2026 08:26
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.

Translation sync: improve result when no Pull Request is necessary

4 participants