-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Continuation of adding translations to theme #778
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
Conversation
This builds on top of #405, addressing the outstanding review feedback. It: * Moves workflow to our standard Transifex workflow, drops recommendation for running babel commands by hand * Configures Transifex * Moves all of the commands needed to maintain translations into Grunt * Sets up docs for translation testing * Covers installation in docs better * Drops recommendation for installation through submodules * Drops superfluous translation documentation * Cleans up some of the code * Updates a lot of related documentation * Updates files at Transifex and brings in full translations back to the translation files in the repository
ericholscher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good with a quick glance. I didn't test it out too much or anything.
| # Spanish translations for sphinx_rtd_theme. | ||
| # Copyright (C) 2018 Read the Docs | ||
| # English translations for sphinx_rtd_theme. | ||
| # Copyright (C) 2019 ORGANIZATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ORGANIZATION is probably not correct
Co-Authored-By: Jesse Tan <[email protected]>
Add back git installation docs, we'll discuss this separately.
|
Okay. I have wrapped up review feedback here, made some changes to prep for the upcoming Gruntfile removal, and have #793 prepped to show what #405 and #778 look like merged together. Both PRs are sizeable now and are ready for merge. IF the last of my changes here look okay, and there is nothing in #405 worth addressing, #793 is ready for merge now. The main feedback I had on #405 was too much documentation and an unclear workflow because of this. If the translation workflow is clear enough, I'm super ready to ship this. We can tweak things as we go and the process is tested more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as far as I can test without Transifex rights, with comments:
- The
consolecode blocks in docs are missing the prompt indicator$which makes the colors dim. I think we discussed this, but I can't find the issue. - Perhaps we should add translation to "Copyright" at the bottom of the page and a translatable alt text on the "Home" icon in breadcrumbs (formerly the word "Docs")
|
Closing, #793 is merged |
This builds on top of #405, addressing the outstanding review feedback. It:
running babel commands by hand
translation files in the repository
After initial review, I'll worry about rebasing all of this on master. For now, let's merge into #405