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

fix: Add french translation missing #20061

Merged
merged 16 commits into from
Aug 30, 2022

Conversation

aehanno
Copy link
Contributor

@aehanno aehanno commented May 13, 2022

SUMMARY

Adding french translation

Add Information

Fixes #20060

@aehanno aehanno mentioned this pull request May 13, 2022
5 tasks
Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

Thanks for the contributon @aehanno . I would recommend splitting this PR into multiple separate PRs, as there are many unrelated changes here. Specifically:

  • The additional French translations (completely valid)
  • Fixing the untranslated strings, like "Home" (this is completely valid)
  • using gettext instead of lazy_gettext (this is probably ok, but should be addressed in a separate PR, perhaps fixing other simiar instances)
  • Translating variables (these won't work)
  • Using moment to calculate the lastModified value fron changed_on instead of changed_on_humanized + adding last_run (this is controversial, but we can probably have that discussion in a separate PR)

Comment on lines 256 to 257
label: t('Sort by %s', label),
label: t('Sort by %s', t(label)),
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right; you can't translate variables like this (the original code looks correct)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR: #20080

Comment on lines 224 to 225
lastModified={cellData.changed_on_humanized}
lastModified={moment.utc(cellData.changed_on).fromNow()}
Copy link
Member

Choose a reason for hiding this comment

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

This should not be changed (@dpgaspar can comment in more detail, but IIRC, the changed_on property needs to be constructed in the backend right now)

Copy link
Contributor Author

@aehanno aehanno May 16, 2022

Choose a reason for hiding this comment

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

changed_on_humanized has no translation. I will be glade to keep 'changed_on_humanized' but what can I change so that the translation is done

I move the change on this PR : #20082
we can discuss about that on the new PR

Comment on lines 75 to 76
lastModified={slice.changed_on_humanized}
lastModified={moment.utc(slice.changed_on).fromNow()}
Copy link
Member

Choose a reason for hiding this comment

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

same here

@@ -314,7 +314,7 @@ export default function DateFilterLabel(props: DateFilterControlProps) {
<Divider />
<div>
<div className="section-title">{t('Actual time range')}</div>
{validTimeRange && <div>{evalResponse}</div>}
{validTimeRange && <div>{t(evalResponse)}</div>}
Copy link
Member

Choose a reason for hiding this comment

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

Again, you can't translate a variable (please see e.g. https://stackoverflow.com/questions/34579316/flask-babel-how-to-translate-variables for an explanation why this is the case)

Copy link
Contributor Author

@aehanno aehanno May 16, 2022

Choose a reason for hiding this comment

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

change has been move in this PR : #20080
We can discuss about translation of varaible in the ohter PR
Without the t() here, there is no translation, do you have an idea, where I can translate this

superset-frontend/src/views/CRUD/welcome/Welcome.tsx Outdated Show resolved Hide resolved
@aehanno
Copy link
Contributor Author

aehanno commented Jun 6, 2022

Hello @villebro,
I created 4 other pr from this one
I pinned you on each one, I hope to get your feedback

@mathieudruart
Copy link
Contributor

mathieudruart commented Jun 24, 2022

Hi @villebro Is there anything we can do to help move these PRs forward? Thanks!

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding these missing translations!

@villebro
Copy link
Member

@mathieudruart it appears you need to fix some linting errors in the translation file:

image

@codecov
Copy link

codecov bot commented Jul 12, 2022

Codecov Report

Merging #20061 (5f1c6ec) into master (523bd8b) will decrease coverage by 0.48%.
The diff coverage is 77.24%.

❗ Current head 5f1c6ec differs from pull request most recent head 3d41d90. Consider uploading reports for the commit 3d41d90 to get more accurate results

@@            Coverage Diff             @@
##           master   #20061      +/-   ##
==========================================
- Coverage   66.55%   66.07%   -0.49%     
==========================================
  Files        1692     1732      +40     
  Lines       64802    65886    +1084     
  Branches     6657     6753      +96     
==========================================
+ Hits        43129    43532     +403     
- Misses      19973    20636     +663     
- Partials     1700     1718      +18     
Flag Coverage Δ
hive ?
mysql ?
postgres 80.99% <ø> (-0.96%) ⬇️
presto ?
python 81.07% <ø> (-1.31%) ⬇️
sqlite 79.60% <ø> (-2.12%) ⬇️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...art-controls/src/operators/contributionOperator.ts 100.00% <ø> (ø)
...-chart-controls/src/sections/advancedAnalytics.tsx 14.28% <0.00%> (-19.05%) ⬇️
...art-controls/src/sections/annotationsAndLayers.tsx 100.00% <ø> (ø)
...perset-ui-chart-controls/src/sections/sections.tsx 100.00% <ø> (ø)
.../shared-controls/components/RadioButtonControl.tsx 0.00% <ø> (ø)
...ges/superset-ui-core/src/query/buildQueryObject.ts 100.00% <ø> (ø)
...ges/superset-ui-core/src/query/extractTimegrain.ts 100.00% <ø> (ø)
...kages/superset-ui-core/src/query/getMetricLabel.ts 100.00% <ø> (ø)
...kages/superset-ui-core/src/query/processFilters.ts 100.00% <ø> (ø)
...superset-ui-core/src/query/types/PostProcessing.ts 100.00% <ø> (ø)
... and 463 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@aehanno
Copy link
Contributor Author

aehanno commented Jul 14, 2022

hey @villebro, correction of workflow has been made, it's possible to launch it ?
Thanks

@mathieudruart
Copy link
Contributor

@mathieudruart it appears you need to fix some linting errors in the translation file:

image

Hi @villebro it should be fixed, is it possible to rerun the workflows ? Thanks !

@aehanno aehanno requested a review from villebro August 11, 2022 18:53
@mathieudruart
Copy link
Contributor

Hi @villebro do you know when this PR will be merged ? thank you !

@villebro villebro merged commit 944808a into apache:master Aug 30, 2022
@villebro
Copy link
Member

Hey @mathieudruart , sorry for the delay; merged!

eschutho pushed a commit that referenced this pull request Sep 20, 2022
AAfghahi pushed a commit that referenced this pull request Oct 5, 2022
AAfghahi pushed a commit that referenced this pull request Oct 6, 2022
@mistercrunch mistercrunch added 🍒 2.0.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XL v2.0 v2.0.1 🍒 2.0.1 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing French Translation
5 participants