Skip to content
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

Move source-code fonts to new build workflow #13093

Merged
merged 15 commits into from
Dec 11, 2024

Conversation

meel-hd
Copy link
Contributor

@meel-hd meel-hd commented Nov 21, 2024

Proposed changes

Moved source-code fonts to new build workflow

Changes Include:

  • Removed the old scripts/yarn/** dir, as it is no longer needed.
  • Moved source-code-pro and source-sans to client/
  • Built and bundled the fonts.
  • Updated templates to use the new built fonts and styles.
  • Added new built bundles license info in REUSE.toml.

Note:

  • The old bundles did not get removed to keep this PR simple for review.

TODO:

  • Remove old fonts bundles.
  • Update any dependents on them to use the new bundles.
  • if FONTS_CDN_URL is configured it should be seted up with static/js/vendor/fonts instead of the old font-source/
  • Remove the old yarn-update script and its configs and workflows

Related: #12172

Checklist

  • Lint and unit tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added documentation to describe my feature.
  • I have squashed my commits into logic units.
  • I have described the changes in the commit messages.

Other information

Changes Include:
- Removed the old scripts/yarn/** dir, as it is no longer needed.
- Moved source-code-pro and source-sans to client/
- Built and bundled the fonts.
- Updated templates to use the new built fonts and styles.
- Added new built bundles license info in REUSE.toml.

Note:
- The old bundles did not get removed to keep this PR simple for review.

TODO:
- Remove old fonts bundles.
- Update any dependents on them to use the new bundles.
- if FONTS_CDN_URL is configured it should be seted up with static/js/vendor/fonts instead of the old font-source/
- Remove the old yarn-update workflow and its configs and workflows

Related: WeblateOrg#12172
@meel-hd meel-hd requested a review from nijel November 21, 2024 10:06
weblate/static/js/vendor/fonts/fonts.css Outdated Show resolved Hide resolved
weblate/templates/snippets/meta-css.html Outdated Show resolved Hide resolved
scripts/yarn-update Outdated Show resolved Hide resolved
nijel added a commit to WeblateOrg/meta that referenced this pull request Nov 21, 2024
nijel added a commit that referenced this pull request Nov 25, 2024
This avoids silent breakages on changes like #13093.
Copy link
Member

@nijel nijel left a comment

Choose a reason for hiding this comment

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

The docs/conf.py also needs to be changed to use the new location of the fonts. I've made the docs build fail when the fonts directory doesn't exist in 425131e.

client/webpack.config.js Outdated Show resolved Hide resolved
- Update docs/conf.py
- Update test_selenium.py
@meel-hd
Copy link
Contributor Author

meel-hd commented Nov 26, 2024

@nijel I rerun the test suite on main (without the new changes) and it seems they fail also in there, I'm not sure what to do next.

summary

weblate/trans/tests/test_selenium.py::SeleniumTests::test_add_component missing element #list-add-button
weblate/trans/tests/test_selenium.py::SeleniumTests::test_fonts missing element #upload_font_submit
weblate/trans/tests/test_selenium.py::SeleniumTests::test_login missing element #dashboard-return
weblate/trans/tests/test_selenium.py::SeleniumTests::test_screenshots missing element #query-dropdown
weblate/trans/tests/test_selenium.py::SeleniumTests::test_weblate

docs/conf.py Outdated Show resolved Hide resolved
@nijel nijel linked an issue Dec 7, 2024 that may be closed by this pull request
11 tasks
@nijel nijel added this to the 5.9 milestone Dec 7, 2024
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.95%. Comparing base (8b7609f) to head (c12d6a5).
Report is 354 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #13093      +/-   ##
==========================================
- Coverage   90.97%   90.95%   -0.03%     
==========================================
  Files         599      601       +2     
  Lines       62058    62390     +332     
  Branches     6397     6453      +56     
==========================================
+ Hits        56457    56744     +287     
- Misses       3944     3974      +30     
- Partials     1657     1672      +15     
Files with missing lines Coverage Δ
weblate/fonts/utils.py 92.00% <ø> (-2.00%) ⬇️
weblate/trans/tests/test_selenium.py 96.09% <ø> (ø)

... and 56 files with indirect coverage changes

@nijel nijel self-assigned this Dec 11, 2024
@nijel nijel merged commit b66d223 into WeblateOrg:main Dec 11, 2024
31 of 33 checks passed
@nijel
Copy link
Member

nijel commented Dec 11, 2024

Merged, thanks! I've made a fix to the testsuite (and reverted unnecessary changes there).

@nijel
Copy link
Member

nijel commented Dec 11, 2024

And I didn't notice messed up licensing info, fixed that now in 1781de2.

@meel-hd meel-hd deleted the move-source-fonts-to-new-build-workflow branch December 11, 2024 12:30
@nijel
Copy link
Member

nijel commented Dec 11, 2024

I've created a followup issue at #13255. Using webpack is better than what we had before, but we might need something different in the end.

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.

JavaScript libraries bundling
2 participants