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.

Sign up for GitHub

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

Jump to bottom

feat(website): Introduction to textlint #986

Merged
merged 16 commits into from
Dec 6, 2023
Merged

feat(website): Introduction to textlint #986

merged 16 commits into from
Dec 6, 2023
+1,236 −77

Conversation

yossydev
Copy link
Contributor

@yossydev yossydev commented Dec 1, 2023 •
edited

This PR is not yet perfect!
Please review the rules I have added and if there are no problems, I will fix the errors and push it again!

Summary

Other OSS translation projects and the textlint rules used in them

  • Next.js
    • textlint-rule-preset-ja-technical-writing
    • textlint-rule-max-comma
    • textlint-rule-preset-ja-spacing
  • sveltekit & svelte-site-jp
    • textlint-rule-detect-bad-chars
    • textlint-rule-no-mix-dearu-desumasu
    • textlint-rule-preset-jtf-style
  • React
    • textlint-rule-preset-jtf-style
  • Vue
    • textlint-rule-detect-bad-chars
    • textlint-rule-no-mix-dearu-desumasu
    • textlint-rule-preset-jtf-style
  • Vite
    • textlint-rule-preset-jtf-style

I have installed textlint-rule-preset-jtf-style which is the most used of them all!

Test Plan

  • CI is successful.
  • No problem with the rules you set.

@github-actions github-actions bot added the A-Website Area: website label Dec 1, 2023
yossydev
yossydev commented Dec 1, 2023
View reviewed changes
website/.textlintrc.json Outdated
Comment on lines 9 to 16
"1.1.3.箇条書き": false,
"1.2.1.句点(。)と読点(、)": false,
"1.2.2.ピリオド(.)とカンマ(,)": false,
"3.1.1.全角文字と半角文字の間": false,
"4.1.1.句点(。)": false,
"4.2.1.感嘆符(！)": false,
"4.2.2.疑問符(？)": false,
"4.3.7.山かっこ<>": false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is taken from react's textlint rule

https://github.com/reactjs/ja.react.dev/blob/30d3e7c9a6169f130b11420d9466c18be5eba5b5/textlint/.textlintrc.json#L6-L13

@yossydev yossydev marked this pull request as ready for review December 1, 2023 02:36
nissy-dev
nissy-dev requested changes Dec 2, 2023
View reviewed changes
Copy link
Member

@nissy-dev nissy-dev left a comment

Choose a reason for hiding this comment

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

Thank you for your investigation! I left some comments, please check it.

@unvalley I don't have personal opinions about settings. If you have , please give some advice about settings like prh.yaml etc.

.github/workflows/website_deploy_preview.yml Outdated
Comment on lines 55 to 57
- name: Run textlint
working-directory: website
run: pnpm textlint
Copy link
Member

Choose a reason for hiding this comment

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

I would like you to create a new file like "ja-translation.yaml" and add this step to that file.
Also, please update the codeowner file for "ja-translation.yaml", ".textlintrc.json" and "prh.yaml".

ref: https://github.com/biomejs/biome/blob/main/.github/CODEOWNERS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review!
feat: CI for running Japanese translation checks in textlint ← added CI for running Japanese translation checks in textlint!

Copy link
Member

@unvalley unvalley Dec 4, 2023 •
edited

Choose a reason for hiding this comment

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

Can this file diff be removed?

Copy link
Contributor Author

@yossydev yossydev Dec 5, 2023 •
edited

Choose a reason for hiding this comment

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

@unvalley
Sorry! I left it as it was...!

remove: Run textlint from website deploy ci ← fixed in

website/package.json Outdated Show resolved Hide resolved
@netlify Netlify
Copy link

netlify bot commented Dec 3, 2023 •
edited

✅ Deploy Preview for biomejs ready!

Name Link
🔨 Latest commit 0d565e0
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/656f31507e6bea000831dc57
😎 Deploy Preview https://deploy-preview-986--biomejs.netlify.app/ja/guides/getting-started
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@yossydev yossydev requested a review from a team as a code owner December 3, 2023 13:56
@github-actions github-actions bot added A-CLI Area: CLI A-Core Area: core A-Project Area: project A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools A-LSP Area: language server protocol L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS L-JSON Language: JSON and super languages A-Diagnostic Area: diagnostocis A-Changelog Area: changelog and removed A-Core Area: core A-Project Area: project A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools A-LSP Area: language server protocol L-CSS Language: CSS A-Diagnostic Area: diagnostocis labels Dec 3, 2023
yossydev
yossydev commented Dec 4, 2023
View reviewed changes
Copy link
Contributor Author

@yossydev yossydev left a comment •
edited

Choose a reason for hiding this comment

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

Now that the rules have been decided, pn textlint:fix has been used to resolve the syntax errors that were currently there 🙏

fix: formatting by textlint:fix

website/.textlintrc.json
"4.2.1.感嘆符(！)": false,
"4.2.2.疑問符(？)": false,
"4.3.7.山かっこ<>": false,
"4.3.2.大かっこ［］": false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "4.3.2. Large brackets []" rule was causing the caution markdown to collapse, so we have added an additional off

biome/website/src/content/docs/ja/guides/getting-started.mdx

Lines 28 to 30 in 6739a51

:::note[注]
Biomeをローカルではなくグローバルにインストールすることも可能ですが、推奨されていません。
:::

@unvalley
Copy link
Member

unvalley commented Dec 5, 2023

@yossydev After resolving this
#986 (comment) it would be good to me.

unvalley
unvalley reviewed Dec 5, 2023
View reviewed changes
.github/workflows/website_deploy_preview.yml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

@yossydev
I think we don't need this workflow file, because we are currently using netlify. And netlify provides the preview feature by itself.

unvalley
unvalley approved these changes Dec 5, 2023
View reviewed changes
Copy link
Member

@unvalley unvalley left a comment

Choose a reason for hiding this comment

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

Thank you!

@unvalley
Copy link
Member

unvalley commented Dec 5, 2023

@nissy-dev please merge this after checking your requested changes🙂

nissy-dev
nissy-dev approved these changes Dec 6, 2023
View reviewed changes
Copy link
Member

@nissy-dev nissy-dev left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@nissy-dev nissy-dev merged commit 7719951 into biomejs:main Dec 6, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nissy-dev nissy-dev nissy-dev approved these changes

@unvalley unvalley unvalley approved these changes

Assignees
No one assigned
Labels
A-CLI Area: CLI A-Website Area: website L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yossydev @unvalley @nissy-dev