Add GitHub workflow to check Fluent reference files #5647
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5647 +/- ##
=======================================
Coverage 85.63% 85.63%
=======================================
Files 312 312
Lines 30867 30867
Branches 8490 8490
=======================================
Hits 26434 26434
Misses 4009 4009
Partials 424 424 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Looks good to me overall! Sorry for the delay due to conference!
Aside from the comments I mentioned below, it might also make sense to put the config.yml and requirements.txt file into a fluent subdirectory. So they can live in places like .github/fluent/linter_config.yml and .github/fluent/requirements.txt.
.github/workflows/fluent_linter.yml
Outdated
| - main | ||
| pull_request: | ||
| paths: | ||
| - 'en/**.ftl' |
There was a problem hiding this comment.
Do we have 'en/**.ftl' path? I think 'locales/en-US/*.ftl' covers it, so we can remove this one.
There was a problem hiding this comment.
One of the many things I missed when adapting an existing workflow. Starting from scratch would have been cleaner.
| - name: Set up Python 3 | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: '3.11' | ||
| cache: 'pip' |
There was a problem hiding this comment.
From what I understand reading this, cache: pip won't work as expected for us here because the requirements.txt file is not in the root directory. You need to specify cache-dependency-path also to make sure that it caches it correctly.
There was a problem hiding this comment.
Input cache-dependency-path is used for cases when multiple dependency files are used, they are located in different subdirectories or different files for the hash that want to be used.
We only use one requirement file, unless I missed some Python used elsewhere?
There was a problem hiding this comment.
My understanding was that this was used when multiple dependency files are present or they are located in different subdirectories. So to be able to cache, the dependency file has to be in the root directly. Otherwise you need to specify the location since it doesn't look for the sub directories by default.
I also see that you can use pip-install: -r requirements.txt here, but I don't know what benefits it adds. Probably not a huge deal.
There was a problem hiding this comment.
Ah see another text here:
Action will try to search dependencies files (requirements.txt for pip and Pipfile.lock for pipenv) in the repository root (or relative to the repository root, if patterns are used) and throw error if no one is found.
There was a problem hiding this comment.
Action will try to search dependencies files (requirements.txt for pip and Pipfile.lock for pipenv) in the repository root (or relative to the repository root, if patterns are used) and throw error if no one is found.
That is why I had to create a requirements.txt file. Installing directly from pip broke the workflow
2b7b8a0
See actions/setup-python#807 The alternative (creating an empty file) seems worse than using an actual requirements file.
Changes: [Nazım Can Altınova] Use getEmptySourceTable instead of constructing the table manually (#5641) [Florian Quèze] Source view for marker stacks (#5633) [Nazım Can Altınova] Add fallback to funcTable.lineNumber when computing line timings (#5644) [Florian Quèze] Show the marker keys instead of the labels in marker tooltips when alt is pressed. (#5625) [Francesco Lodolo] Add GitHub workflow to check Fluent reference files (#5647) [Nazım Can Altınova] Switch from Gitpod to GitHub Codespaces (#5653) [Nazım Can Altınova] Migrate from CircleCI to GitHub Actions (#5654) [Nazım Can Altınova] Run the CI in the production branch and on every pull request (#5661) [Nazım Can Altınova] Convert l10n-sync.js from Flow comment types to JSDoc comments for Typescript (#5659) [Florian Quèze] Display correct unit in Stack Chart tooltips for size profiles. (#5658) [Florian Quèze] Hide track management UI in single track profiles. (#5656) [Florian Quèze] Rename hiddenTrackCount to trackCount. (#5665) [Florian Quèze] Cleanup the profile info panel for size profiles (show file size and name, hide empty sections). (#5657) [Markus Stange] Use valibot for symbolication API response validation (#5666) [Nazım Can Altınova] Make sure to fetch the full git history for l10n-sync job in CI (#5667) [Nazım Can Altınova] Do not switch to call tree when clicking the activity graph while on sample based panels (#5672) And thanks to our localizers: be: Mikalai Udodau de: Michael Köhler el: Jim Spentzos en-GB: Ian Neal es-CL: ravmn fr: Théo Chevalier fy-NL: Fjoerfoks ia: Melo46 it: Francesco Lodolo [:flod] nl: Mark Heijl pt-BR: Marcelo Ghelman ru: berry, Valery Ledovskoy sv-SE: Andreas Pettersson tr: Grk zh-CN: Olvcpr423, wxie zh-TW: Pin-guang Chen
Fixes #5646