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

CIの導入 #19

Merged
merged 7 commits into from
Jan 4, 2025
Merged

CIの導入 #19

merged 7 commits into from
Jan 4, 2025

Conversation

newt239
Copy link
Member

@newt239 newt239 commented Jan 4, 2025

GitHub ActionsでPRの作成時に以下の4つをチェックするCIを追加

  • TypeScriptのコンパイルエラー
  • ESLint
  • Prettier
  • Stylelint

@newt239
Copy link
Member Author

newt239 commented Jan 4, 2025

ESLintの設定を修正し、リンターを走らせました

Comment on lines 8 to 9
- name: Checkout main branch
uses: actions/checkout@v4
Copy link
Contributor

@Meiryo7743 Meiryo7743 Jan 4, 2025

Choose a reason for hiding this comment

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

8 行目について,PR の base branch に対しても実行されることを考えると「Checkout main branch」の「main」を「target」のような表現に変えてみるのはどうでしょうか? それなら意図が一層明瞭になって良さそうです

次行のような各 Actions についても,セキュリティーの観点からバージョンは commit hash による指定が望ましいです。pinact のようなツールを使って機械的に置換すればサクッと対応できます。願わくば Renovate を使ってこの更新管理も自動化できるとなお良しではあるものの,流石にそこは本件の範囲外といったところでしょうかね……。

Suggested change
- name: Checkout main branch
uses: actions/checkout@v4
- name: Checkout target branch
uses: actions/checkout@ # commit hash に置き換える

Copy link
Member Author

Choose a reason for hiding this comment

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

どちらも納得です、対応します!

@newt239 newt239 requested a review from Meiryo7743 January 4, 2025 09:49
Copy link
Contributor

@Meiryo7743 Meiryo7743 left a comment

Choose a reason for hiding this comment

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

LGTM ですー!

@newt239 newt239 merged commit bebd7c6 into main Jan 4, 2025
1 check passed
@newt239 newt239 deleted the feature/ci-cd branch January 4, 2025 11:50
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.

2 participants