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

Publish report of translation status. #2190

Merged
merged 2 commits into from
Jul 8, 2024
Merged

Publish report of translation status. #2190

merged 2 commits into from
Jul 8, 2024

Conversation

qwandor
Copy link
Collaborator

@qwandor qwandor commented Jul 4, 2024

No description provided.

@qwandor qwandor force-pushed the report branch 2 times, most recently from 9fdd69b to 39f6473 Compare July 4, 2024 17:48
@qwandor qwandor marked this pull request as ready for review July 8, 2024 08:07
@qwandor qwandor requested review from mgeisler and djmitche July 8, 2024 08:13
@qwandor qwandor enabled auto-merge (squash) July 8, 2024 08:33
Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

It doesn't look like this report is linked anywhere? What is it? I see https://github.com/google/mdbook-i18n-helpers/tree/main/i18n-report but there's no documentation about it.

@qwandor
Copy link
Collaborator Author

qwandor commented Jul 8, 2024

I'll add links in a separate PR once I'm sure it's working properly. It's a report showing the status of each of the different language translations of the course. You can try generating it locally yourself with i18n-report report.html po/*.po if you want to see it before this PR is merged.

Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

OK, sounds good. Please also add some docs in the mdbook-i18n-helpers repo?

@qwandor qwandor merged commit c39b6b0 into main Jul 8, 2024
36 checks passed
@qwandor qwandor deleted the report branch July 8, 2024 14:51
@@ -67,6 +67,7 @@ Then install these tools with:
cargo install mdbook
cargo install --locked mdbook-svgbob
cargo install --locked mdbook-i18n-helpers
cargo install --locked i18n-report
Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically, it's only the GitHub action that needs this tool, right? So we could avoid mentioning it in the README and thus save people a bit of time when setting up the tooling.

Since https://crates.io/i18n-report is currently empty, the command will actually fail right now.

@qwandor, would you be up for renaming the crate to something more specific to Gettext/PO files? Now that I see it in an install command, the name "i18n report" seems a bit too generic and all-encompassing :-)

Perhaps we could call it gettext-report or po-stats or similar? The nearest equivalent in the Gettext suite it msgfmt --stat.

Copy link
Collaborator Author

@qwandor qwandor Jul 10, 2024

Choose a reason for hiding this comment

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

Technically, it's only the GitHub action that needs this tool, right? So we could avoid mentioning it in the README and thus save people a bit of time when setting up the tooling.

People might want to run it locally to see the status of translations after their changes, so I figured it was best to put the installation instructions here with the rest of the tools.

Since https://crates.io/i18n-report is currently empty, the command will actually fail right now.

Do you mean https://crates.io/crates/i18n-report? I've sent google/mdbook-i18n-helpers#213 to add a readme, but in any case it will still install fine.

@qwandor, would you be up for renaming the crate to something more specific to Gettext/PO files? Now that I see it in an install command, the name "i18n report" seems a bit too generic and all-encompassing :-)

Perhaps we could call it gettext-report or po-stats or similar? The nearest equivalent in the Gettext suite it msgfmt --stat.

Those do both seem like reasonable names, but I'm reluctant to rename it after already publishing it to crates.io, as it would be confusing to have two names for the same thing show up when people search for it there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since https://crates.io/i18n-report is currently empty, the command will actually fail right now.

Do you mean https://crates.io/crates/i18n-report? I've sent google/mdbook-i18n-helpers#213 to add a readme, but in any case it will still install fine.

Oh, sorry, I meant the latter. I was too fast in my typing 😄

Perhaps we could call it gettext-report or po-stats or similar? The nearest equivalent in the Gettext suite it msgfmt --stat.

Those do both seem like reasonable names, but I'm reluctant to rename it after already publishing it to crates.io, as it would be confusing to have two names for the same thing show up when people search for it there.

Yes, I see... I guess this name is okay then and it's not like there are a lot of demand for these crate names.

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.

3 participants