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

[JA] Proofreading on GitHub Actions #185

Merged
merged 18 commits into from
May 19, 2020
Merged

[JA] Proofreading on GitHub Actions #185

merged 18 commits into from
May 19, 2020

Conversation

sfujiwara
Copy link
Contributor

@sfujiwara sfujiwara commented Apr 11, 2020

@lamberta
Japanese reviewers: @ohtaman, @masa-ita, @AseiSugiyama, @yukinagae, @nuka137, @chie8842, @kiszk

This PR is a proposal of #144

This PR allows us to automatically apply proofreading tool on GitHub Actions.
It would be a great help for Japanese document reviewers and translators.

I'm happy to hear your opinion about this PR!

Files and directories

.github/workflows/ja.yaml

This workflow runs only when directories and files below are changed:

on:
  push:
    paths:
      - ".github/workflows/ja.yaml"
      - "site/ja/**"
      - "tools/ja/**"

tools/ja/

This is a proofreading tool used in Japanese community.

Resulting outputs

We can see the result of proofreading on GitHub PR.
The CI would fail if there are too many errors:

image

We can find the detailed results in GitHub Actions log:

image

@sfujiwara sfujiwara requested a review from lamberta as a code owner April 11, 2020 08:32
@googlebot googlebot added the cla: yes CLA has been signed label Apr 11, 2020
@sfujiwara sfujiwara changed the title Feature/GitHub actions ja [JA] Proofreading on GitHub Actions Apr 11, 2020
@lamberta
Copy link
Member

Nice! I'll have a look through this week and will test some things.

- name: Set up Python
uses: actions/setup-python@v1
with:
python-version: "3.7"
Copy link
Member

Choose a reason for hiding this comment

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

Make the python version 3.6.

Google is at this version so we can;t test anything beyond 3.6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I fixed it!

@lamberta
Copy link
Member

Hi @sfujiwara , do you know if redpen has a list of supported languages somewhere? I recall it not working for Korean, are there any other restrictions?

You are using this to maintain a translation glossary, yes? (I'm just wondering if other communities can use this setup, as well)

@kiszk
Copy link

kiszk commented Apr 25, 2020

@lamberta This PR intentionally (and temporary) drop supporting Korean due to CI failure. There is an issue to support Korean again. But, it have not been closed yet.

@sfujiwara
Copy link
Contributor Author

@lamberta RedPen has various kinds of validators for example sentence length, invalid words, and so on. Some validators does not support Korean. But, other validators supports all languages including Korean.

I guess RedPen and GitHub Actions are useful for other communities!
See the official document for details:
http://redpen.cc/docs/1.10/index.html#validator

@sfujiwara
Copy link
Contributor Author

Hi, how is it going on this PR?

For example:

```shell script
./tools/ja/bin/proofreading.sh site/ja/guide
Copy link
Member

Choose a reason for hiding this comment

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

Tested this locally and seems to work ... I used homebrew to install the redpen package, probably out of scope to provide detailed install instructions, but maybe mention the Java dependency---speaking of Java, will this run in the GitHub Actions VM?

There was a log of noise at the command-line when running this, is that usual? Any way to filter out all these deprecation warnings?

@lamberta
Copy link
Member

Thanks for the bump on this @sfujiwara (been swamped in email lately).

Tested locally and over here: lamberta#6
and seems to work as advertised.

As long as this is restricted to the ja/ translations then seems fine. Long-term, I'll need your help to maintain this and update if there are structural changes to the docs (things move around). If that's ok with you, then we can merge.

@sfujiwara
Copy link
Contributor Author

@lamberta OK. We will maintain and improve the proofreading tool to make our translation tasks efficient!

@sfujiwara
Copy link
Contributor Author

Hi @lamberta

There was a log of noise at the command-line when running this, is that usual? Any way to filter out all these deprecation warnings?

I added a minor change for the above proposal.

  • Separate RedPen logs to files and make it available as GitHub Actions artifacts
  • Add logback.xml to set RedPen log level Warn on GitHub Actions

Merge_pull_request__2_from_sfujiwara_fix-logs_·_sfujiwara_docs-l10n_3c6aca9

speaking of Java, will this run in the GitHub Actions VM?

Currently it works on my repository and GitHub Actions (I will delete this repository if this PR is merged):
https://github.com/sfujiwara/docs-l10n/actions/runs/107112341

@lamberta
Copy link
Member

Thanks @sfujiwara for the updates.

@MarkDaoust and @yashk2810 if there's no objections, I'll merge this for the ja folks. If there are maintenance concerns or changes needed, we can loop in @sfujiwara

@MarkDaoust
Copy link
Member

LGTM.

Copy link
Member

@lamberta lamberta left a comment

Choose a reason for hiding this comment

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

Sounds good. We'll keep an eye on it and will loop you in if there ae any issues. Thanks!

@lamberta lamberta merged commit 934135c into tensorflow:master May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA has been signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants