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

ci: lint commit messages #862

Merged
merged 1 commit into from
Dec 11, 2023
Merged

ci: lint commit messages #862

merged 1 commit into from
Dec 11, 2023

Conversation

sasaplus1
Copy link
Member

@sasaplus1 sasaplus1 commented Dec 11, 2023 •
edited
Loading

課題

#858 (comment) のように人間がコミットメッセージについてレビューする必要がある

対応内容

commitlintがインストールされるようpackage.jsonに記述されているのでそれを利用してCIで検査させる

期待する状態

CIにある程度機械的にコミットメッセージを検査させ、人間が楽をする

@sasaplus1 sasaplus1 self-assigned this Dec 11, 2023
@sasaplus1 sasaplus1 marked this pull request as ready for review December 11, 2023 02:23
@itsminadesu itsminadesu requested review from a team, herablog and tokimari and removed request for a team December 11, 2023 02:52
@yossydev
Copy link
Member

yossydev commented Dec 11, 2023 •
edited
Loading

@sasaplus1
これってすでにローカルのコミット時に検査されるようになっていると思っているのですが(認識違っていたらすみません！！ 🙏)
プラスα CIで検査させたい！という認識であっていますでしょうか?? 👀

@sasaplus1
Copy link
Member Author

huskyが入ってるはずなので検査されてて然るべしだと思うんですが、人によって環境構築が正常にできていないのかそれともフックの類を実行しない環境になっているのか--no-verifyでコミットしているのか、スルーされることがあるのでCIでやっておいた方がいいか、という感じです。ブラウザでGitHubから直接コミットした際にもCIでチェックされますし。

@yossydev
Copy link
Member

yossydev commented Dec 11, 2023 •
edited
Loading

人によって環境構築が正常にできていないのかそれともフックの類を実行しない環境になっているのか--no-verifyでコミットしているのか、スルーされることがあるのでCIでやっておいた方がいいか、という感じです。

なるほど！二重って感じですね 👀

ブラウザでGitHubから直接コミットした際にもCIでチェックされますし。

:tasikani:

yossydev
yossydev approved these changes Dec 11, 2023
View reviewed changes
Copy link
Member

@yossydev yossydev left a comment

Choose a reason for hiding this comment

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

LGTMeow

@itsminadesu
Copy link
Contributor

itsminadesu commented Dec 11, 2023 •
edited
Loading

課題
#858 (comment) のように人間がコミットメッセージについてレビューする必要がある

上記の指摘は「fix: hogehogeじゃなくてfix(spindle-ui): hogehogeのように（スコープがある場合は）スコープ付きにしてください」という指摘だったため、本PRの変更自体では上記の指摘はカバーできなさそうです... 🙏🏻

itsminadesu
itsminadesu approved these changes Dec 11, 2023
View reviewed changes
Copy link
Contributor

@itsminadesu itsminadesu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ただ、（ローカル同様）CI上でもcommitlintが走るようにした方が良いのは確かになので、本PRの内容は非常に助かります...！ご対応いただきありがとうございます！ 🙇🏻

@sasaplus1
Copy link
Member Author

#862 (comment)

@commitlint/config-lerna-scopes が入ってるので多分検査されると思います。されなかったら設定がおかしいか不具合か、スコープを間違えているかいずれかな気がします。

@sasaplus1
Copy link
Member Author

ああ、スコープがなくてもコミットができるという意味か。

@sasaplus1 sasaplus1 merged commit eac46aa into main Dec 11, 2023
7 checks passed
@sasaplus1 sasaplus1 deleted the ci/lint-commit-message branch December 11, 2023 09:00
@itsminadesu
Copy link
Contributor

ああ、スコープがなくてもコミットができるという意味か。

ですです〜

@kc7891 kc7891 mentioned this pull request Dec 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yossydev yossydev yossydev approved these changes

@itsminadesu itsminadesu itsminadesu approved these changes

@kc7891 kc7891 Awaiting requested review from kc7891

@herablog herablog Awaiting requested review from herablog herablog was automatically assigned from openameba/spindle-working-group

@tokimari tokimari Awaiting requested review from tokimari tokimari was automatically assigned from openameba/spindle-working-group

Assignees

@sasaplus1 sasaplus1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sasaplus1 @yossydev @itsminadesu