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

Clean up translations list #1119

Merged
merged 18 commits into from May 2, 2024
Merged

Conversation

safwansamsudeen
Copy link
Contributor

@safwansamsudeen safwansamsudeen commented Jun 13, 2023

Replaces and closes #897.

As discussed, I have moved the docs links to the Language column, have the names link to email addresses and GH usernames link to GH profiles. I've also added commas separating the items in the Links column and standardized the Transifix link.

I've added a few names of the language managers. For Korean, when I added the name 오동권, the table didn't format, for some reason. For those whom I haven't linked the email ID or GH profile, I couldn't find either.

There was a recent merge conflicting with this one: what's the difference between RTD and the normal docs? As #1114 had both in the Turkish row, I've maintained that. I've used Git for years and still mess up merging, so excuse me for the mess.

Is there anything else I could change?


📚 Documentation preview 📚: https://cpython-devguide--1119.org.readthedocs.build/documentation/translating/

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Jun 13, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@CAM-Gerlach CAM-Gerlach changed the title gh-#897: Clean up translations Clean up translations list Jun 14, 2023
@CAM-Gerlach CAM-Gerlach self-requested a review June 14, 2023 07:25
documentation/translating.rst Outdated Show resolved Hide resolved
documentation/translating.rst Outdated Show resolved Hide resolved
documentation/translating.rst Outdated Show resolved Hide resolved
Co-authored-by: Ezio Melotti <[email protected]>
@safwansamsudeen
Copy link
Contributor Author

safwansamsudeen commented Jun 14, 2023

Thanks for the review!

What's causing the lint failure?

@ezio-melotti
Copy link
Member

It's likely a bug in sphinx-lint, since it doesn't play well with tables: https://github.com/sphinx-contrib/sphinx-lint#known-issues
In theory it should detect and skip tables, but it looks like it's not working in this instance. I'm taking a look at the issue to see if I can put together a quick fix.

@hugovk
Copy link
Member

hugovk commented Jun 14, 2023

Maybe switch to the list style format? Would make future maintenance easier too, as there's no need to shuffle the spacing next time.

@ezio-melotti
Copy link
Member

This was discussed in #897 (comment).
I now created a PR to fix the sphinx-lint issue: sphinx-contrib/sphinx-lint#65

@hugovk
Copy link
Member

hugovk commented Jun 14, 2023

Fair enough :) List style would be easy to edit online via the GitHub UI (desktop and phones), Notepad, BBEdit, vim etc. (Although I bet vim has some plugin :) Anyway, thanks for the sphinx-lint PR and table style is fine.

@safwansamsudeen
Copy link
Contributor Author

@CAM-Gerlach, just bumping this. I'm not sure after how long we're supposed to remind you (in other OSS projects I read that you can tag reviewers after a week), so do correct me in this regard.

@hugovk
Copy link
Member

hugovk commented Jun 17, 2023

We need the CI to pass before we can merge.

That means either waiting for sphinx-contrib/sphinx-lint#65 to be merged, released, and updated in this repo. Or switching to list style.

@ezio-melotti
Copy link
Member

sphinx-contrib/sphinx-lint#65 got merged, but it's not released yet.

@willingc willingc added the needs: PR update An update or rebase to an existing PR is needed. label Oct 10, 2023
@hugovk
Copy link
Member

hugovk commented Nov 18, 2023

We need the CI to pass before we can merge.

That means either waiting for sphinx-contrib/sphinx-lint#65 to be merged, released, and updated in this repo. Or switching to list style.

@safwansamsudeen This is now done, please can you resolve the conflicts in this PR? Thanks!

@safwansamsudeen
Copy link
Contributor Author

@hugovk I'm busy this week, but I think I'll be able to do it by the end of the month. I hope that's alright.

@hugovk
Copy link
Member

hugovk commented Nov 18, 2023

Perfectly alright, no rush at all! Thank you!

@hugovk
Copy link
Member

hugovk commented May 2, 2024

@safwansamsudeen Hi, checking in to see if you've time to look at this again? Thanks!

@safwansamsudeen
Copy link
Contributor Author

@hugovk I'm sorry, but I don't think I'll be able to continue working on this.

@hugovk
Copy link
Member

hugovk commented May 2, 2024

That's okay, thanks for your work so far!

@hugovk
Copy link
Member

hugovk commented May 2, 2024

Ready for review!

I switched the table to list-table because I was struggling getting the indent right and getting links to work over newlines.

I added a gihub-user role to the extlinks.

And I adjusted some of the contact details, see last few commit messages.

Before: https://devguide.python.org/documentation/translating/

After: https://cpython-devguide--1119.org.readthedocs.build/documentation/translating/

@hugovk
Copy link
Member

hugovk commented May 2, 2024

One question: should we add a link in the "Language" column for all the translations which are on docs.python.org, even if they're not in the dropdown menu?

For example, right now https://devguide.python.org/documentation/translating/ has https://docs.python.org/pl/ but it's not in the dropdown.

But it also doesn't have https://docs.python.org/uk/ (also not in the dropdown).

Co-authored-by: Ezio Melotti <[email protected]>
@ezio-melotti
Copy link
Member

One question: should we add a link in the "Language" column for all the translations which are on docs.python.org, even if they're not in the dropdown menu?

I don't see why not. Translators might want to check that out, and the link will make it easier for them to access it, especially if it's not listed in the dropdown.

@hugovk hugovk merged commit b9eda58 into python:main May 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: PR update An update or rebase to an existing PR is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants