Skip to content

Split out names list invariants - #1611

Merged
eggrobin merged 3 commits into
unicode-org:mainfrom
eggrobin:names-list-invariants
Nov 2, 2026
Merged

eggrobin merged 3 commits into
unicode-org:mainfrom
eggrobin:names-list-invariants

Conversation

@eggrobin

@eggrobin eggrobin commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Since the names list gets updated asynchronously, they can fail.

  • Approver: Feel free to merge on my behalf
    • rebase & merge one or more commits
    • squash & merge multiple commits into one

@eggrobin
eggrobin requested a review from markusicu November 2, 2026 15:41
Comment on lines +450 to +452
# Out-of-source build.
nameslist:
name: NamesList data invariants

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.

Each steps seems to do what its name suggests, but it's not totally clear to me how this all hangs together and why we need so many steps to run these tests.
Would you mind adding some comment lines here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added comments. Most of this is setup from https://github.com/unicode-org/unicodetools/blob/main/docs/build.md; setting up the tools is somewhat messy.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

At some point it might make sense to factor this out into our own actions, but for now it gets copy-pasted ad nauseam.

repository: unicode-org/unicodetools
path: unicodetools/mine/src
- name: Checkout base UnicodeData.txt
if: ${{ github.event_name == 'pull_request'}}

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.

why is this condition on some of the steps? can we make it a condition on the whole task?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Explained in the comments: this is used to turn some errors into notices, but we want to run the tests unconditionally.

repository: unicode-org/unicodetools
path: unicodetools/mine/src
- name: Checkout base UnicodeData.txt
if: ${{ github.event_name == 'pull_request'}}

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.

what if the condition is false / it's not a PR?
do we just skip checking out the base file, or do we stop the task?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We just skip this step.

set +e
diff base-repertoire.txt merged-repertoire.txt
echo "REPERTOIRE_CHANGED=$?" >> "$GITHUB_ENV"
- name: Get the CLDR_REF from pom.xml

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.

why do we need CLDR for these tests?
what is CLDR_REF? a branch name? what does it do for us here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think nothing works without CLDR (and then the CLDR_REF thing is just putting it at the right revision).

@eggrobin
eggrobin merged commit 3ff0e2b into unicode-org:main Nov 2, 2026
18 checks passed
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.

2 participants