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を設定 #38

Merged
merged 7 commits into from Aug 19, 2023
Merged

CIを設定 #38

merged 7 commits into from Aug 19, 2023

Conversation

Percy08-dev
Copy link
Collaborator

No description provided.

@cotton-alta cotton-alta self-requested a review August 15, 2023 14:15
Copy link
Collaborator

@cotton-alta cotton-alta left a comment

Choose a reason for hiding this comment

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

いくつかコメントしました:pray:

.github/workflows/actions.yml Outdated Show resolved Hide resolved
@@ -1,29 +1,42 @@
name: frontend_build
run-name: ${{ github.actor }} is learning GitHub Actions
on: [release]

on: [push]
jobs:
build_frontend:
if: ${{ always() }} # 無条件にjobを実行
Copy link
Collaborator

Choose a reason for hiding this comment

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

[question]
こちらのalwaysはおそらくですが、backendのjobが追加された場合を想定したものですよね?(そうであればこのままで大丈夫です:ok_hand:)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

そうですね
ただよく考えると、片方しか更新されてないのに両方でCIが走るのはどうなの?って思ったのでフロント、バックそれぞれ更新があった方にだけCIが走るように書き直します。
https://blog.35d.jp/2020-09-29-github-actions-path

- name: Install dependencies
run: pnpm install --frozen-lockfile


# [5] ビルドチェック(linterが在れば要らない?)
- name: build check
run: pnpm build
Copy link
Collaborator

Choose a reason for hiding this comment

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

build checkはあるんですが、lintがないようです:eyes:
一応VSCode上でPrettierによる保存時フォーマットは有効になってはいると思うものの、
CIでもチェックしたほうがいいと思うのですがどうでしょうか?

@para7

Copy link
Collaborator

@para7 para7 Aug 16, 2023

Choose a reason for hiding this comment

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

@cotton-alta
確かに。入れるべきかもしれません

@Percy08-dev
後ほど実行コマンドお送りします

Copy link
Collaborator

Choose a reason for hiding this comment

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

今日フロント課題のセッティング等の方をやってたので明日コマンド作成します…

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Percy08-dev
お待たせしました。
先ほど新しくプルリクを発行してマージしましたので、取り込んで以下の実行をお願いします!

pnpm ci/lint

- name: catalog build
run: pnpm ci:ladle
run: pnpm ci/ladle
Copy link
Collaborator

Choose a reason for hiding this comment

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

こちら申し訳ないのですが、github pagesが無料プランだと使えないようなので、一旦コメントアウトしておいてもらえますか:bow:(カタログのデプロイ先を別issueで検討したいと思います)

合わせて以下のようにTODOコメントも追記しておいていただけるとありがたいです:pray:

# TODO: https://github.com/stlatica/stlatica/issues/39

Copy link
Collaborator

Choose a reason for hiding this comment

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

@cotton-alta
プライベートリポジトリだから、ですよねたぶん。
ブランチ保護もそうですし、公開にして解決するなら将来的に公開予定ですしもう出しちゃってもいいのではないかという気もしてきたのですがどうなんでしょう。

Copy link
Collaborator

Choose a reason for hiding this comment

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

公開にしちゃいますかー
discordの方で一応問題ないか確認してから変更させてください:pray:

Copy link
Collaborator

Choose a reason for hiding this comment

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

こちら申し訳ないのですが、github pagesが無料プランだと使えないようなので、一旦コメントアウトしておいてもらえますか🙇(カタログのデプロイ先を別issueで検討したいと思います)

こちらの問題は解決したので、忘れてもらって大丈夫です

Copy link
Collaborator

@cotton-alta cotton-alta left a comment

Choose a reason for hiding this comment

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

LGTMです:tada:対応ありがとうございます!!

2つほどsuggested changeがあるのですが、問題なさそうであればマージ前に取り込んじゃってください:pray:(commit suggestionで取り込めます)

- uses: actions/setup-go@v4
with:
go-version: '1.20.6'
cache: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

- name: catalog build
run: pnpm ci:ladle
run: pnpm ci/ladle
Copy link
Collaborator

Choose a reason for hiding this comment

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

こちら申し訳ないのですが、github pagesが無料プランだと使えないようなので、一旦コメントアウトしておいてもらえますか🙇(カタログのデプロイ先を別issueで検討したいと思います)

こちらの問題は解決したので、忘れてもらって大丈夫です

.github/workflows/actions.yml Outdated Show resolved Hide resolved
.github/workflows/actions.yml Outdated Show resolved Hide resolved
@cotton-alta cotton-alta changed the title フロントエンドのCIを設定 CIを設定 Aug 19, 2023
Co-authored-by: cotton <chinoknct@gmail.com>
@Percy08-dev Percy08-dev merged commit 08a918f into main Aug 19, 2023
3 checks passed
@Percy08-dev Percy08-dev deleted the Percy08-dev-CI-setting branch August 19, 2023 15:41
@cotton-alta cotton-alta mentioned this pull request Aug 19, 2023
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.

None yet

3 participants