Skip to content

Commit

Permalink
Reuse server strings when possible in stats (#3442)
Browse files Browse the repository at this point in the history
* Reuse server strings when possible in stats

We were making people translate stuff twice a lot more than strictly
needed, and mwiencek suggested we don't. I'm sure I've missed some
reused strings, but this is most of them.

To ensure we don't accidentally use l for statistics strings,
this explicitly imports l as lMbServer and uses that.
We have a test to prevent bare l(), but it doesn't seem to work
at the moment since it wasn't triggered by this.

* Fix "Checking translation domain in statistics code" test

 1. The `!` negation does not actually cause the script to exit (despite `set -e`)
    if the `git grep` succeeds (finds matches). See [1]:

    "The shell does not exit if the command that fails is part of the command
     list immediately following a while or until keyword, part of the test in
     an if statement, part of any command executed in a && or || list except
     the command following the final && or ||, any command in a pipeline but
     the last, or if the command’s return status is being inverted with !."

    Thus I've replaced the `!` at the beginning of the command with
    `&& { exit 1; }` at the end.

 2. It appears that the tests could hang on the `git grep` command if it
    produced too many matches due to the results being paged. I've also added
    the `--no-pager` flag to resolve this.

[1] https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html:

* Use more standard l_mb_server name

This was blocked by eslint but we have exceptions for all others,
and looks a lot more readable than lMbServer (and related names)

* Add missing `*l*_mb_server` function names to .eslintrc.unfixed.yaml

These aren't used yet, but it seems likely they will be in the future, so it
shouldn't hurt to add them.

* Add providePluginConfig for `*l*_mb_server` functions

---------

Co-authored-by: Michael Wiencek <[email protected]>
  • Loading branch information
reosarevok and mwiencek authored Jan 21, 2025
2 parents fe32554 + 4b04fb2 commit 14cf92b
Show file tree
Hide file tree
Showing 14 changed files with 179 additions and 140 deletions.
6 changes: 6 additions & 0 deletions .eslintrc.unfixed.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ rules:
"l_languages",
"ln_languages",
"lp_languages",
"l_mb_server",
"ln_mb_server",
"lp_mb_server",
"l_relationships",
"ln_relationships",
"lp_relationships",
Expand All @@ -34,6 +37,9 @@ rules:
"N_l",
"N_ln",
"N_lp",
"N_l_mb_server",
"N_ln_mb_server",
"N_lp_mb_server",
"N_l_statistics",
"N_lp_statistics",
"__webpack_public_path__",
Expand Down
4 changes: 3 additions & 1 deletion docker/musicbrainz-tests/run_circleci_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ sudo -E -H -u musicbrainz ./node_modules/.bin/eslint --max-warnings 0 .
echo OK

echo Checking translation domain in statistics code
! sudo -E -H -u musicbrainz git grep -Pw '(N_)?l[np]?\(' -- 'root/statistics/**.js'
sudo -E -H -u musicbrainz \
git --no-pager grep -Pw '(N_)?l[np]?\(' -- 'root/statistics/**.js' \
&& { exit 1; }
echo OK

sv_start_if_down chrome
Expand Down
3 changes: 3 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ export default [
'l_languages',
'ln_languages',
'lp_languages',
'l_mb_server',
'lp_mb_server',
'l_relationships',
'ln_relationships',
'lp_relationships',
Expand All @@ -234,6 +236,7 @@ export default [
'N_l',
'N_ln',
'N_lp',
'N_l_mb_server',
'N_l_statistics',
'N_lp_statistics',
'__webpack_public_path__',
Expand Down
12 changes: 6 additions & 6 deletions root/statistics/Countries.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,27 @@ component Countries(
<tr>
<th className="pos">{l_statistics('Rank')}</th>
<th>
{l_statistics('Country')}
{l_mb_server('Country')}
<div className="arrow" />
</th>
<th>
{l_statistics('Artists')}
{l_mb_server('Artists')}
<div className="arrow" />
</th>
<th>
{l_statistics('Releases')}
{l_mb_server('Releases')}
<div className="arrow" />
</th>
<th>
{l_statistics('Labels')}
{l_mb_server('Labels')}
<div className="arrow" />
</th>
<th>
{l_statistics('Events')}
{l_mb_server('Events')}
<div className="arrow" />
</th>
<th>
{l_statistics('Places')}
{l_mb_server('Places')}
<div className="arrow" />
</th>
<th>
Expand Down
6 changes: 3 additions & 3 deletions root/statistics/Editors.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ component Editors(
<StatisticsLayout
fullWidth
page="editors"
title={l_statistics('Editors')}
title={l_mb_server('Editors')}
>
<p>
{texp.l_statistics('Last updated: {date}', {date: dateCollected})}
Expand All @@ -101,13 +101,13 @@ component Editors(
<EditorStatsTable
countLabel={l_statistics('Open and applied edits in past week')}
dataPoints={topRecentlyActiveEditors}
editorLabel={l_statistics('Editor')}
editorLabel={l_mb_server('Editor')}
tableLabel={l_statistics('Most active editors in the past week')}
/>
<EditorStatsTable
countLabel={l_statistics('Total applied edits')}
dataPoints={topEditors}
editorLabel={l_statistics('Editor')}
editorLabel={l_mb_server('Editor')}
tableLabel={l_statistics('Top editors overall')}
/>
</div>
Expand Down
2 changes: 1 addition & 1 deletion root/statistics/Edits.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ component Edits(
<p>
{texp.l_statistics('Last updated: {date}', {date: dateCollected})}
</p>
<h2>{l_statistics('Edits')}</h2>
<h2>{l_mb_server('Edits')}</h2>
{Object.keys(statsByCategory).length === 0 ? (
<p>
{l_statistics('No edit statistics available.')}
Expand Down
6 changes: 3 additions & 3 deletions root/statistics/Formats.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ component Formats(
<thead>
<tr>
<th className="pos">{l_statistics('Rank')}</th>
<th>{l_statistics('Format')}</th>
<th>{l_statistics('Releases')}</th>
<th>{l_mb_server('Format')}</th>
<th>{l_mb_server('Releases')}</th>
<th>{l_statistics('% of total releases')}</th>
<th>{l_statistics('Mediums')}</th>
<th>{l_mb_server('Mediums')}</th>
<th>{l_statistics('% of total mediums')}</th>
</tr>
</thead>
Expand Down
Loading

0 comments on commit 14cf92b

Please sign in to comment.