Skip to content

Conversation

yasulab
Copy link
Member

@yasulab yasulab commented Sep 14, 2025

#1704 does not update news section automatically because it does not have a workflow to update DB in production. So this PR fixes it.

📰 DojoLetter has Vol.88 but not updated in the news section

The news section: https://coderdojo.jp/#news

image

DojoLetter: https://news.coderdojo.jp/category/dojoletterバックナンバー/

image

@yasulab yasulab changed the title [WIP] Fix: Failed to update news section automatically Fix: Failed to update news section automatically Sep 14, 2025
@yasulab
Copy link
Member Author

yasulab commented Sep 14, 2025

@claude Review my PR?

Copy link
Contributor

claude bot commented Sep 14, 2025

Claude finished @yasulab's task —— View job


タスクリスト

  • リポジトリ固有の指示(CLAUDE.md)を確認
  • 新しいワークフローファイル(daily.yml)を読む
  • 削除されたワークフローファイル(fetch_news.yml)を読む
  • 変更内容を分析してレビュー
  • フィードバックを提供

PRレビュー

このPRは、ニュースセクションの自動更新が機能しない問題(#1704)を修正するものですね。ワークフロー名をfetch_news.ymlからdaily.ymlに変更し、本番環境へのデプロイステップを追加することで問題を解決しようとしています。

🚨 重大な問題 (セキュリティ、破壊的変更)

1. ワークフロー内のジョブ定義エラー(57-64行目)

daily.ymlの53-64行目で、runs-onが二重に定義されています:

# 53行目: 最初のジョブで定義
- name: 🚀 Deploy to Heroku if news found
  if: ${{ env.FOUND_NEWS == 'true' }}
  # TODO: This workflows depend on Ubuntu version.
  # https://github.com/AkhileshNS/heroku-deploy/issues/186
  runs-on: ubuntu-22.04  # ❌ これはステップ内で使えません
  steps:  # ❌ ステップ内でstepsは定義できません

修正案:別ジョブとして定義するか、単一のステップとして修正する必要があります。

# オプション1: 単一ステップとして修正
- name: 🚀 Deploy to Heroku if news found
  if: ${{ env.FOUND_NEWS == 'true' }}
  uses: akhileshns/[email protected]
  with:
    heroku_api_key:  ${{ secrets.HEROKU_API_KEY }}
    heroku_app_name: ${{ secrets.HEROKU_APP_NAME }}
    heroku_email:    ${{ secrets.HEROKU_EMAIL }}

2. mainブランチへの直接push(36-40行目)

git checkout main
git add db/news.yml
# ...
git push origin main

mainブランチへの直接pushは危険です。保護されたブランチの場合、このワークフローは失敗します。

⚠️ 改善推奨 (ベストプラクティス違反)

1. 不要なbundle installの重複(27行目)

bundler-cache: trueを使用している場合、明示的なbundle installは不要です:

- name: 💎 Setup Ruby
  uses: ruby/setup-ruby@v1
  with:
    bundler-cache: true  # これで自動的にbundle installされます

- name: 🧪 Install dependencies
  run: bundle install  # ❌ 重複して実行されます

2. git configでユーザー情報のハードコード(34-35行目)

個人情報をハードコードせず、GitHub Actionsのbot accountを使用することを推奨:

# 現在
git config user.name  "Yohei Yasukawa"
git config user.email "[email protected]"

# 推奨
git config user.name  "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
💡 提案 (より良い実装方法)

1. 環境変数の出力方法の改善

GitHub Actionsの推奨方法を使用:

# 現在のコード
echo "FOUND_NEWS=true"  >> $GITHUB_ENV

# より明確な書き方
echo "FOUND_NEWS=true" >> "$GITHUB_ENV"

2. ジョブの分離を検討

デプロイを別ジョブとして定義し、依存関係を明確にする:

jobs:
  daily:
    runs-on: ubuntu-latest
    outputs:
      found_news: ${{ steps.commit.outputs.found_news }}
    steps:
      # ... 現在のステップ ...
      
  deploy:
    needs: daily
    if: ${{ needs.daily.outputs.found_news == 'true' }}
    runs-on: ubuntu-22.04
    steps:
      - uses: actions/checkout@v3
      - uses: akhileshns/[email protected]
        # ...
✅ 良い実装 (参考になる箇所)

1. workflow_dispatchの追加

手動実行を可能にすることで、デバッグとテストが容易になります。

2. 条件付きコミットの実装(38-46行目)

git diff --cached --quietを使用して、実際に変更がある場合のみコミットする実装は良いプラクティスです。

3. 絵文字を使った視覚的なログ

ステップ名やログメッセージに絵文字を使用することで、Actions の実行結果が見やすくなっています。


まとめ

最も重要な修正点は57-64行目のワークフロー構文エラーです。これを修正しないと、ワークフローは実行時にエラーになります。また、mainブランチへの直接pushについても、ブランチ保護設定によっては問題となる可能性があります。

その他の改善点は、コードの保守性と安全性を高めるためのものです。特に個人情報のハードコードは避けることをお勧めします。

主な改善:
- デプロイを独立したジョブに分離し、GitHub Actionsのベストプラクティスに準拠
- job outputs を使用した適切なジョブ間データ共有の実装
- $GITHUB_ENV から $GITHUB_OUTPUT への移行(GitHub推奨の最新仕様)
- 重複した bundle install を削除(bundler-cache: true で自動実行)
- 環境変数名を FOUND_NEWS に統一(一貫性のある大文字表記)
- heroku-deploy アクションを最新版(v3.14.15)に更新

効果:
- ニュースが見つからない場合はデプロイジョブ自体がスキップされ、実行時間を短縮
- ワークフローの構造が明確になり、保守性が向上
- GitHub Actions の最新仕様に準拠し、将来的な互換性を確保
@yasulab yasulab force-pushed the fix-automate-updating-news branch from 30f5d0a to e9f9223 Compare September 14, 2025 07:11
@yasulab yasulab merged commit 9d8fe5c into main Sep 14, 2025
5 checks passed
@yasulab yasulab deleted the fix-automate-updating-news branch September 14, 2025 07:12
yasulab added a commit that referenced this pull request Sep 14, 2025
yasulab added a commit that referenced this pull request Sep 14, 2025
@yasulab
Copy link
Member Author

yasulab commented Sep 14, 2025

Fixed! 🚀✅✨ https://coderdojo.jp/#news

image

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.

1 participant