Closed Pugma closed 3 weeks ago
Here are some key observations to aid the review process:
⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪ |
🧪 No relevant tests |
🔒 No security concerns identified |
⚡ No major issues detected |
/improve
Latest suggestions up to 56a792a
Category | Suggestion | Score |
Best practice |
イメージタグの一意性と追跡可能性を向上させる。___ **IMAGE_TAG を 'main' にハードコードするのではなく、CIプロセスのバージョンやコミットハッシュを使用することで、より一意で追跡可能なタグ付けが可能になります。** [.github/workflows/staging.yml [10]](https://github.com/traPtitech/traPortfolio-UI/pull/230/files#diff-7bb7a67ed05f3845ed1f1b92dc4b87dfb0dd9aba208cca5489055ccef315117bR10-R10) ```diff -IMAGE_TAG: main +IMAGE_TAG: ${{ github.sha }} ``` Suggestion importance[1-10]: 8Why: Using a unique identifier like the commit hash for the image tag enhances traceability and avoids potential conflicts with hardcoded tags. This is a significant improvement for CI/CD processes. | 8 |
ブランチ名のハードコードを避け、設定の柔軟性を高める。___ **ブランチ名をハードコードするのではなく、環境変数や設定ファイルから取得するように変更してください。これにより、将来的にブランチ名が変更された場合にも、設定のみの変更で対応可能となります。** [.github/workflows/main.yml [6]](https://github.com/traPtitech/traPortfolio-UI/pull/230/files#diff-7829468e86c1cc5d5133195b5cb48e1ff6c75e3e9203777f6b2e379d9e4882b3R6-R6) ```diff branches: - - 'main' + - ${{ env.DEFAULT_BRANCH }} ``` Suggestion importance[1-10]: 7Why: The suggestion to use an environment variable for the branch name instead of hardcoding it increases flexibility and maintainability, especially if branch names change in the future. This is a good practice for configuration management. | 7 |
Category | Suggestion | Score |
Enhancement |
___
**`IMAGE_TAG` の値を 'master' から 'main' に更新してください。ブランチ名が変更されたため、タグも対応する必要があります。**
[.github/workflows/staging.yml [10]](https://github.com/traPtitech/traPortfolio-UI/pull/230/files#diff-7bb7a67ed05f3845ed1f1b92dc4b87dfb0dd9aba208cca5489055ccef315117bR10-R10)
```diff
-IMAGE_TAG: master
+IMAGE_TAG: main
```
| 9 |
コードの変更は良さそうです! ありがとうございます
ghcrのレジストリのタグは Ansible と歩調を合わせる必要があるため見送り
これについて質問です ghcrのレジストリのタグを変更する時期は決めていますか?また、変更するのに適している時期などはありますか?
ghcrとAnsibleについては少し調べてみて、ghcrはコンテナイメージ(=アプリケーションとその実行ファイル、設定ファイルなどを一まとめにしたもの)を管理するもので、Ansibleとはインフラ構築自動化・管理ツールであることは分かりました
もともと、 main
にタグを更新する前のものを traP のサーバーで ansible を使って呼び出していると思っててそう書いてました
でも、 ansible では latest
タグを参照していて、 main
はステージング環境用でしかないということに気づいて自己解決したので、
結局更新していない IMAGE_TAG の部分はステージング環境用のものだったので、すぐに反映させられることもあり、変更を反映させておきました
というコメントを残していたつもりでした
ある程度理解できました ありがとうございます!
User description
ブランチ名変更に伴った参照先の変更を実施 ghcrのレジストリのタグは Ansible と歩調を合わせる必要があるため見送り
PR Type
configuration changes
Description
Changes walkthrough 📝
main.yml
CIワークフローのブランチ名変更
.github/workflows/main.yml - ブランチ名を 'master' から 'main' に変更
staging.yml
ステージングワークフローのブランチ名と名称変更
.github/workflows/staging.yml