-
Notifications
You must be signed in to change notification settings - Fork 0
Add Vietnamese #42
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
base: main
Are you sure you want to change the base?
Add Vietnamese #42
Conversation
invalid team IDThe team ID in your (If this PR is already fixing this, ignore the warning. But preferably fix it You can follow this guideline for help. |
1 similar comment
invalid team IDThe team ID in your (If this PR is already fixing this, ignore the warning. But preferably fix it You can follow this guideline for help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the Zalando Sans build outputs and project tooling/config as part of the “Add Vietnamese” update, including a version bump, dependency lockfile generation, and CI/build pipeline tweaks.
Changes:
- Bump font version to
1.800and add updated compiled TTF asset(s). - Replace the placeholder
requirements.txtwith a fully pinned, pip-compiled lockfile. - Update customization script and CI workflow behavior (URL/badge handling, pushing, triggers/permissions, artifact naming).
Reviewed changes
Copilot reviewed 4 out of 175 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
sources/config.yaml |
Bumps the project version to 1.800. |
scripts/customize.py |
Removes raw_url usage, updates badge URL replacement logic, and re-enables git push after customization. |
requirements.txt |
Replaces placeholder with a fully pinned dependency lockfile generated by pip-compile. |
fonts/ttf/ZalandoSans-CondensedExtraLight.ttf |
Adds/updates a built TTF binary artifact (version shows 1.800). |
Makefile |
Adds fontspector badge generation output and minor comment formatting changes. |
.github/workflows/build.yaml |
Expands triggers, changes tool/action setup, adjusts artifact naming, and updates release packaging steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result = git.status("--porcelain") | ||
| if any(line.startswith("M ") for line in result.splitlines()): | ||
| git.commit("-m", "Customize repository") |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git status --porcelain can return statuses other than staged modifications (M ), e.g. newly added files (A ) or untracked files (??). With the current any(line.startswith("M ") ...) check, the script can incorrectly print "Nothing changed" and skip committing/pushing even though there are changes staged by this script. Consider checking for any non-empty porcelain output (or explicitly handling A / ?? / D ) instead of only M .
| print("Pushing changes to GitHub") | ||
| git.push() |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Automatically pushing to origin as part of make customize is risky/unexpected (it may fail without credentials, and it removes the user’s chance to review the changes before publishing). Consider prompting the user, making the push opt-in via a flag/env var, or leaving push to a separate explicit step.
| tags: | ||
| - '*' | ||
|
|
||
| permissions: write-all |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
permissions: write-all grants this workflow broad write access to the repository and other resources. This is significantly more privilege than is required for building/testing and uploading artifacts, and it increases the blast radius if any step/action is compromised. Prefer the principle of least privilege (e.g. default contents: read, and only add pages: write / id-token: write in the specific job/step that needs it).
| permissions: write-all | |
| permissions: | |
| contents: read |
| - uses: actions/checkout@v6 | ||
| - name: Set up Python 3.11 | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: "3.10" | ||
| python-version: "3.11" |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI installs Python 3.11 here, but requirements.txt is generated with Python 3.13 (per its header). If the lockfile includes Python-version-specific resolution, the environment created in CI may diverge from what was locked. Consider aligning the CI Python version with the version used for pip-compile, or regenerating the lock with 3.11 to match CI.
|
The build is working locally, no idea why it doesn't work here through the github action |
Here is the specimen and diff with previous version:
260210_Diff.zip
260210_Specimen.zip