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

lint: Apply lint only to modified files #136

Merged
merged 3 commits into from
Dec 2, 2024
Merged

Conversation

s3-odara
Copy link
Contributor

@s3-odara s3-odara commented Dec 1, 2024

編集、追加されたファイルをtextlint_flagsに設定して、プルリクエスト時にそれらだけlintが行われるようにする。

@s3-odara
Copy link
Contributor Author

s3-odara commented Dec 2, 2024 via email

Copy link
Member

@akiomik akiomik left a comment

Choose a reason for hiding this comment

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

Go Bold

@s3-odara s3-odara merged commit 8cd7fb5 into nostr-jp:main Dec 2, 2024
1 check passed
@SnowCait
Copy link
Member

SnowCait commented Dec 2, 2024

tsuyoshicho/action-textlint は reviewdog ベースだからデフォルトで差分見るようになってない?
https://github.com/tsuyoshicho/action-textlint/blob/41934cc28a5ffb1d5dd499d5d2d2a7c516c38bd0/action.yml#L18-L22

@s3-odara
Copy link
Contributor Author

s3-odara commented Dec 2, 2024

なるほど?
revertの機運が

@s3-odara
Copy link
Contributor Author

s3-odara commented Dec 2, 2024

linterから送られた結果をフィルターするようにも見えるんですけどどうなんでしょう。 (PR意図としてはlintを早く終わらせたい)
https://github.com/reviewdog/reviewdog?tab=readme-ov-file#filter-mode

@SnowCait
Copy link
Member

SnowCait commented Dec 2, 2024

なるほど、実行速度を上げたいのね。
アクション reviewdog の中身までは確認してないから事前にフィルターすることで速くなる可能性はありそうだけど、この変更を入れても 40s 台でそんなに変わってなさそうかも?
この PR: https://github.com/nostr-jp/nips-ja/actions/runs/12115951963/job/33775357463
別の PR: https://github.com/nostr-jp/nips-ja/actions/runs/12115902585/job/33775206492

@s3-odara
Copy link
Contributor Author

s3-odara commented Dec 2, 2024

うーん。github actionsのログの見方よくわからないですけどmodified filesって名前のjobがないのは構文エラーとかでこの変更が意図したように動いてないってことなんですかね。

1年ぐらい前だとlintは30秒台で終わることが多くて、これは校正対象が増えたからな気がしてます。
https://github.com/nostr-jp/nips-ja/actions?page=18

@s3-odara
Copy link
Contributor Author

s3-odara commented Dec 2, 2024

action_textlintの下にwithでtextlint_flags付けたら早くなった
環境変数の渡し方がまずいのかな
https://github.com/s3-odara/nips-ja/actions/runs/12122501663

@SnowCait
Copy link
Member

SnowCait commented Dec 2, 2024

ああこれ pull_request_target だから main ブランチのワークフローで動いてるのか。
tsuyoshicho/action-textlint のドキュメントに特に環境変数について記載がないから with で渡す必要ありそう。

@s3-odara
Copy link
Contributor Author

s3-odara commented Dec 2, 2024

これで動いたんですけど、これだとブランチ名でクローンしてくるからPR元のmainブランチに別のコミットがあったりすると問題がありそうなので一旦保留
これと同じような問題かも https://zenn.dev/hkusu/articles/c731862051438b

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.

3 participants