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: Correction translation #28434

Closed

Conversation

aehanno
Copy link
Contributor

@aehanno aehanno commented May 10, 2024

Correction french translation

SUMMARY

Remove to fuzzy to add the right translation

BEFORE

image

TESTING INSTRUCTIONS

Check that after we got the new translation

Correction translation
@aehanno aehanno requested review from villebro and rusackas as code owners May 10, 2024 19:00
@github-actions github-actions bot added i18n Namespace | Anything related to localization i18n:french Translation related to French language labels May 10, 2024
Copy link

codecov bot commented May 10, 2024

Codecov Report

Attention: Patch coverage is 83.41232% with 35 lines in your changes are missing coverage. Please review.

Project coverage is 67.27%. Comparing base (76d897e) to head (f8bd651).
Report is 109 commits behind head on master.

Files Patch % Lines
...t-frontend/src/explore/actions/saveModalActions.ts 87.62% 7 Missing and 5 partials ⚠️
superset/views/base.py 82.35% 9 Missing ⚠️
superset/migrations/shared/utils.py 25.00% 6 Missing ⚠️
superset/views/database/views.py 81.48% 5 Missing ⚠️
superset/db_engine_specs/presto.py 50.00% 2 Missing ⚠️
...-frontend/src/features/alerts/AlertReportModal.tsx 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #28434      +/-   ##
==========================================
+ Coverage   60.48%   67.27%   +6.79%     
==========================================
  Files        1931     1944      +13     
  Lines       76236    77305    +1069     
  Branches     8568     8671     +103     
==========================================
+ Hits        46114    52010    +5896     
+ Misses      28017    23171    -4846     
- Partials     2105     2124      +19     
Flag Coverage Δ
hive ?
javascript 57.73% <87.00%> (+0.01%) ⬆️
mysql 77.12% <80.18%> (?)
postgres 77.23% <80.18%> (?)
presto ?
python 77.43% <80.18%> (+13.94%) ⬆️
sqlite 76.68% <80.18%> (?)
unit ?

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aehanno aehanno changed the title Correction translation fix: Correction translation May 10, 2024
@github-actions github-actions bot added i18n:spanish Translation related to Spanish language i18n:italian Translation related to Italian language i18n:chinese Translation related to Chinese language i18n:japanese Translation related to Japanese language i18n:russian Translation related to Russian language i18n:korean Translation related to Korean language i18n:dutch i18n:slovak i18n:ukrainian i18n:portuguese i18n:brazilian i18n:traditional-chinese labels May 10, 2024
@aehanno
Copy link
Contributor Author

aehanno commented May 13, 2024

As it is link to this issue #28431

@rusackas, Do you know what I can do about the Docker error? I didn't modify that part, so I don't understand why I'm getting this error

Thanks

@mistercrunch
Copy link
Member

Assuming we would:

  • add to docker build
  • add to official pypi release process
  • adapt docs/build instructions

I guess that leaves people building from the repo directly out

From the perspective of pypi/docker it's not a breaking change. For the rest, maybe a note in UPDATING.md suffice?

@rusackas
Copy link
Member

For others' context, there's a somewhat parallel discussion happening here:
#25988

I think it's fair to remove the .mo/.json files if the're auto-generated at a time that supports as many use cases as possible. I'm not sure if dockery/pypi cover all the bases one would expect.

If this were to be done at start time, I think it's pretty quick, but it might be possible to even do it conditionally depending on configuration... then those who have translations enabled always run the freshest translations, no matter how they install/run Superset.

dependabot bot and others added 3 commits May 13, 2024 15:44
@aehanno
Copy link
Contributor Author

aehanno commented May 14, 2024

@rusackas It will impact what I did ?
Do I need to change something on my PR
Because I change the po files then with the po2json I generated the json file

@rusackas
Copy link
Member

@aehanno I think your PR is OK... we're just trying to figure out what to do with the json files, and it's just kind of happening on this thread randomly.

@rusackas
Copy link
Member

rusackas commented May 14, 2024

Actually @aehanno, in looking at the PR files, there are a whole lot of changed files that look like they have nothing to do with translations. Maybe the rebase didn't work right?

@aehanno
Copy link
Contributor Author

aehanno commented May 14, 2024

Actually @aehanno, in looking at the PR files, there are a whole lot of changed files that look like they have nothing to do with translations. Maybe the rebase didn't work right?

I did another merge and rebase but I still got those change
I don't know why

The diff is weird beacause if we check the first file for example, that say I change the version from 4.0.0 to 4.0.1 but I checked the same file in master branch and I got 4.0.1

@rusackas
Copy link
Member

Yeah, something here isn't right. I suspect you're merging master onto your feature branch rather than rebasing. @mistercrunch might have the git-fu to get us out of this mess, but here's a randomly-slected article on the difference that miiiight help.

@aehanno
Copy link
Contributor Author

aehanno commented May 14, 2024

@rusackas Can be a solution to create a new branch then the pull request ? Instead of did the correction on this one

@rusackas
Copy link
Member

@aehanno absolutely. I've definitely done it several times :D

@aehanno aehanno closed this May 14, 2024
@aehanno aehanno deleted the add-new-translation-french branch May 14, 2024 17:48
@aehanno
Copy link
Contributor Author

aehanno commented May 14, 2024

New PR here #28497

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies:npm github_actions Pull requests that update GitHub Actions code i18n:brazilian i18n:chinese Translation related to Chinese language i18n:dutch i18n:french Translation related to French language i18n:italian Translation related to Italian language i18n:japanese Translation related to Japanese language i18n:korean Translation related to Korean language i18n:portuguese i18n:russian Translation related to Russian language i18n:slovak i18n:spanish Translation related to Spanish language i18n:traditional-chinese i18n:ukrainian i18n Namespace | Anything related to localization packages risk:db-migration PRs that require a DB migration size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.