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

docs: non-checkpoint-branch workflow #1855

Merged
merged 13 commits into from
Dec 15, 2023

Conversation

Jinjiang
Copy link
Member

@Jinjiang Jinjiang commented May 22, 2023

An update to follow this workflow demo:
https://github.com/Jinjiang/translation-workflow-demo

This is considered an alternative approach to #1824

Changes:

  1. Included the changes in Add translation status on page top #1824
  2. Added a new npm script to generate a file named translation-status.json to record translation checkpoints. This script should be run when each time we sync up translations for any language.
  3. Simplified the workflow by removing the maintenance of checkpoint branches.

Thanks.

@netlify
Copy link

netlify bot commented May 22, 2023

Deploy Preview for vue-router canceled.

Name Link
🔨 Latest commit 23bab9a
🔍 Latest deploy log https://app.netlify.com/sites/vue-router/deploys/657bd1b3c4f2c20008414643

@codecov-commenter
Copy link

codecov-commenter commented May 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0b90a50) 90.77% compared to head (23bab9a) 90.77%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1855   +/-   ##
=======================================
  Coverage   90.77%   90.77%           
=======================================
  Files          24       24           
  Lines        1116     1116           
  Branches      348      348           
=======================================
  Hits         1013     1013           
  Misses         63       63           
  Partials       40       40           

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

@Jinjiang Jinjiang force-pushed the jinjiang/docs/non-checkpoint-branch-workflow branch 5 times, most recently from 130e1e6 to e285522 Compare August 27, 2023 17:07
@Jinjiang
Copy link
Member Author

@posva This is ready for review. Thanks.

@posva
Copy link
Member

posva commented Aug 28, 2023

@Jinjiang Thanks! Really sorry for the delay, I will give it a look in 1-2 weeks

@nazarepiedady
Copy link
Contributor

@Jinjiang I really liked the idea, I hope it goes ahead.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

Thanks a lot @Jinjiang and sorry again for the delay 🙏

.github/contributing.md Outdated Show resolved Hide resolved
.github/contributing.md Outdated Show resolved Hide resolved
Comment on lines 136 to 140
1. See what translation you need to do to sync up with the original docs. There are 2 popular ways:
1. Via the GitHub Compare page: https://github.com/vuejs/router/compare/ (only see the changes in `packages/docs/*`) from the checkpoint hash to `main` branch. You can find the checkpoint hash for your language via the translation status file `packages/docs/.vitepress/translation-status.json`.
2. Via a local command: `pnpm run docs:compare-to-translate <lang> [<commit>]`.
2. Create your own branch and start the translation update, following the previous comparison.
3. Same to step 4 and step 5 in the “starting a new language” workflow. Once you have done all above, create a pull request and then wait for the maintainers to approve and merge it.
Copy link
Member

Choose a reason for hiding this comment

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

There are a few issues with this section:

  • no introductory sentence. It's as if the 1st point was that but I think it's missing something along the lines of Keeping translations updated
  • inconsistent spacing (3 and 4, should be 2 spaces)
  • The sample URL should be a real example so users can actually replace things in that URL to test it out

Copy link
Member Author

Choose a reason for hiding this comment

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

@posva for spacing, I found 2 spaces in front of ordered list won't be recognized as indents in the Markdown preview. So I kept 4 spaces instead. The 3 spaces was a mistake, sorry about that (however they can also be recognized as indents).

For the rest part I modified a little accordingly. Please take a further look. Thanks.

1. Via the GitHub Compare page: https://github.com/vuejs/router/compare/ (only see the changes in `packages/docs/*`) from the checkpoint hash to `main` branch. You can find the checkpoint hash for your language via the translation status file `packages/docs/.vitepress/translation-status.json`.
2. Via a local command: `pnpm run docs:compare-to-translate <lang> [<commit>]`.
2. Create your own branch and start the translation update, following the previous comparison.
3. Same to step 4 and step 5 in the “starting a new language” workflow. Once you have done all above, create a pull request and then wait for the maintainers to approve and merge it.
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to explicitly add the steps 4 and 5 with adapted commands

.github/contributing.md Outdated Show resolved Hide resolved
packages/docs/generate-translation-status.mjs Outdated Show resolved Hide resolved
packages/docs/generate-translation-status.mjs Outdated Show resolved Hide resolved
packages/docs/generate-translation-status.mjs Outdated Show resolved Hide resolved
packages/docs/generate-translation-status.mjs Outdated Show resolved Hide resolved
packages/docs/generate-translation-status.mjs Outdated Show resolved Hide resolved
@Jinjiang
Copy link
Member Author

Jinjiang commented Oct 9, 2023

@posva hey, do you think we can merge this or there is anything further to improve? :-)

@Jinjiang Jinjiang force-pushed the jinjiang/docs/non-checkpoint-branch-workflow branch from a33d15f to 23bab9a Compare December 15, 2023 04:10
@posva posva merged commit 0187f7e into main Dec 15, 2023
6 checks passed
@Jinjiang Jinjiang deleted the jinjiang/docs/non-checkpoint-branch-workflow branch December 15, 2023 05:24
@Jinjiang
Copy link
Member Author

@nazarepiedady now this workflow has been officially confirmed. feel free to translate the vue router docs in this way. 😉

I will also write something about this in the vuejs-translations org later.

Thanks.

@nazarepiedady
Copy link
Contributor

@Jinjiang, I will start making the necessary changes to organize the work I am doing over the Portuguese translation to match the current conclusions about how the translation workflow should be.

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.

4 participants