-
Notifications
You must be signed in to change notification settings - Fork 4
GitHub Actions #37
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
base: master
Are you sure you want to change the base?
GitHub Actions #37
Conversation
.github/workflows/config.yml
Outdated
runs-on: ubuntu-18.04 | ||
|
||
steps: | ||
- uses: actions/checkout@v1 |
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.
これだとactions/checkout@v1
Dockerコンテナの上でさらにコンテナを起動することになるので、
uses: docker://tfug/proofreading:latest
として run:
の中をbin/run tensorflow/docs master result.txt
としたほうが自然な気がします。
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.
僕もまだ GitHub Actions あんまりちゃんとわかっていないけれど、
uses: actions/checkout@v1
みたいなuse
は処理の単位の区切り (action と呼ぶらしい) で、その単位ごとに任意のイメージを使ったり予め用意した処理を呼び出すことができる- Action を指していたり Docker image そのものを指していたりしてややこしい (?)
actions/checkout
は公式に用意されているやつで CircleCI で言うcheckout
と同じ- なので後ろの RedPen による check とかは
actions/checkout
とは別のコンテナで動かす形になっている- イメージを指定しなければ
ubuntu-latest
上になる
- イメージを指定しなければ
みたいな感じっぽいです。
YAML がわかり辛いなと思ったので、こんな感じで修正してみました:
name: actions
on: [push]
jobs:
build:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Check diff
uses: docker://tfug/proofreading:latest
with:
args: ./bin/run tensorflow/docs master
...
@@ -1,5 +1,7 @@ | |||
# Proofreading for TensorFlow docs translation | |||
|
|||
 |
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.
このCIジョブはすべてのブランチでのpushイベントに対して発火するように設定されていますが、その結果をバッチに出すとmasterはちゃんと動作しているのにfailが表示されてしまうということになり、メンテナンスされていないかのように受け取られます。
このリポジトリ上でGithub Actionsを使って実現できそうなことは以下の3つがあります。(他にもブランチの定期削除とかもできますが)
- PRが出ているブランチに対するテスト
- mergeされたmasterブランチの内容に対するテスト
- on.scheduleを使って定期的にtensorflow/docsのmasterの内容をチェックする
このPRでの内容は上記の 1.
と思ってレビューしましたが、バッチを出すとしたら2.
か3.
の結果に対して出すのが良いかと思います。
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.
たぶん今の状態だとバッジ上のステータスはブランチに関わらずデフォルトブランチである master のステータスが表示されるので、懸念しているような master 以外のブランチで CI 失敗して failed が表示されるということにはならないかと。
https://help.github.com/ja/actions/automating-your-workflow-with-github-actions/configuring-a-workflow#adding-a-workflow-status-badge-to-your-repository
@@ -1,2 +1,3 @@ | |||
# Temporary directory to clone GitHub repository | |||
ghrepos | |||
result.txt |
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.
bin/run
では結果のファイルはresult.txt
でなくても第3引数で指定できるようになっています。
#33 (comment) でコメントしましたが、result.txt
に固定して引数で渡さないようにするのがいいかなと思っています。
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.
ここは #42 で修正済み
master 側で大きい修正が入ったので、いったん master をこっちの branch にマージして、ついでに conflict の解消とかしました
これ RedPen で修正の要求が出ると終了コードが 1 になって CI コケた扱いになってしまうのか |
|
あれ?なんか |
#36 何か CI 的なのがあった方が気分が盛り上がるかなと思って GitHub Actions で
./bin/run-docker
コマンドがちゃんと正しく動くかチェックするようにしてみた