WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@machida
Copy link
Member

@machida machida commented Dec 3, 2025

Summary by CodeRabbit

リリースノート

  • スタイル

    • ルートのCSS変数による背景画像参照を追加し、フロントのスタイル定義を整理しました。
    • スクリプト内で参照していた背景画像の直接設定を削除しました。
  • チョア

    • Sass関連の依存を更新し、ビルド環境で新しいSass実装を採用する設定を導入しました。
  • リファクタ

    • Sass処理の設定をモダンAPIに合わせて調整しました。

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions github-actions bot requested a review from komagata December 3, 2025 15:03
@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

JavaScriptパックから画像インポートと関連のCSS変数設定を削除し、Sassのビルド周りをsass-embeddedへ移行するためwebpack設定とSass関連パッケージを更新。スタイルシート側でルートの背景画像変数を追加。

Changes

Cohort / File(s) Change Summary
パッケージ依存関係
package.json
sass を ^1.69.5 → ^1.83.1 に更新。sass-embedded ^1.83.0 を追加。sass-loader を ^13.3.2 → ^14.2.1 に更新。
webpack 設定
config/webpack/webpack.config.js
全ての sass-loader インスタンスに対して sass-embedded を実装として設定し、api: 'modern' を適用、sassOptions に複数の非推奨警告抑制オプションをマージするロジックを追加。
スタイルシート
app/javascript/stylesheets/lp/base/_base.sass
:root に CSS カスタムプロパティ --people-bg-image を追加(assets/images/background/people.png を参照するURL)。
アセット参照の再構成
app/javascript/packs/lp.js
画像アセットの import 削除と、削除された画像を割り当てていた CSS カスタムプロパティ設定の削除。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • config/webpack/webpack.config.js の sass-embedded 適用ロジックは細部確認が必要(すべてのルールで正しく置換されるか、既存オプションとのマージ挙動)。
  • 依存バージョンアップに伴うビルド互換性と生成されるCSSの差異を確認すること(sass/sass-embedded の挙動差)。
  • app/javascript/packs/lp.js の削除がクライアント側で参照される他箇所に影響を与えないか確認。

Possibly related PRs

Suggested reviewers

  • komagata

Poem

🐰 今日はコードをちょいと整理して、
画像はCSSへそっと収めたよ。
新しいSassの道具を抱えて、
ビルドは軽やかに踊り出す、
にんじん一つ、祝杯に! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning PRの説明が完全に不足しています。テンプレートで要求されるIssue番号、概要、変更確認方法、スクリーンショットなどが一切記載されていません。 テンプレートに従い、Issue番号、変更内容の概要、変更確認方法、変更前後のスクリーンショットを追加してください。
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PRのタイトルは、Sassの警告修正とトップページのメインビジュアル画像表示の問題修正という2つの明確な変更を正確に説明しており、変更内容と完全に一致しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-sass-for-rails-update

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6415cf0 and 44ac6a4.

⛔ Files ignored due to path filters (1)
  • app/javascript/assets/images/background/people.png is excluded by !**/*.png
📒 Files selected for processing (1)
  • app/javascript/stylesheets/lp/base/_base.sass (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/javascript/stylesheets/lp/base/_base.sass
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build_and_test

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
config/webpack/webpack.config.js (1)

27-32: 非推奨警告の抑制は一時的な対応として妥当ですが、将来的な対応が必要です。

4つの非推奨警告(legacy-js-apiimportglobal-builtincolor-functions)を抑制していますが、これはSass 1.83への移行時の一時的な対応として理解できます。ただし、特に以下の項目は将来的にコードを更新すべきです:

  • import: @import@use / @forward に移行
  • global-builtin: ビルトイン関数を名前空間付きで使用
  • color-functions: 新しいcolor関数構文に移行

これらの非推奨機能の使用箇所を追跡するため、issueを作成しますか?または、使用箇所を検索するスクリプトを生成できます。

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dae9279 and 6415cf0.

⛔ Files ignored due to path filters (5)
  • public/icon.png is excluded by !**/*.png
  • public/icon.svg is excluded by !**/*.svg
  • public/images/background/people.png is excluded by !**/*.png
  • public/images/background/people.svg is excluded by !**/*.svg
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (4)
  • app/javascript/packs/lp.js (0 hunks)
  • app/javascript/stylesheets/lp/base/_base.sass (1 hunks)
  • config/webpack/webpack.config.js (1 hunks)
  • package.json (1 hunks)
💤 Files with no reviewable changes (1)
  • app/javascript/packs/lp.js
🧰 Additional context used
📓 Path-based instructions (2)
config/**

📄 CodeRabbit inference engine (AGENTS.md)

Configuration files in config/ should include environment settings, routes, and lints (.rubocop.yml, config/slim_lint.yml)

Files:

  • config/webpack/webpack.config.js
**/*.js

⚙️ CodeRabbit configuration file

**/*.js: - どうしても避けられない時以外に新規にVue.js, Reactのコードを追加しない。

  • fetchメソッドの代わりにrequest.jsを使う。

Files:

  • config/webpack/webpack.config.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:131-135
Timestamp: 2025-09-12T21:18:00.834Z
Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: jun-kondo
Repo: fjordllc/bootcamp PR: 0
File: :0-0
Timestamp: 2025-07-23T20:42:19.974Z
Learning: fjordllc/bootcampプロジェクトでは、hタグ(見出し)の文言は日本語でハードコーディングする方針が確立されており、I18n対応は行わない。例:app/views/welcome/logo.html.slimでh2、h3タグが日本語でハードコーディングされている。
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to app/javascript/**/*.{js,ts,jsx,tsx} : JavaScript/TypeScript code in `app/javascript/` should be linted with ESLint and formatted with Prettier via `yarn lint` scripts; use React 17 and Shakapacker/Webpack 5

Applied to files:

  • package.json
  • config/webpack/webpack.config.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build_and_test
🔇 Additional comments (3)
app/javascript/stylesheets/lp/base/_base.sass (1)

20-24: LGTM!

レスポンシブ対応のスタイリング変更は適切に実装されています。text-blockミックスインの使用とメディアクエリによるブレークポイント処理が正しく行われています。

Also applies to: 26-49, 58-59

package.json (1)

66-68: No known security vulnerabilities found in the specified Sass package versions.

[email protected], [email protected], and [email protected] have no reported CVEs or security advisories. These versions are current (released Jan–Dec 2024) and well-maintained. While sass-loader's version bump (13.x → 14.x) represents a major release, no compatibility conflicts are evident from current security and stability information.

config/webpack/webpack.config.js (1)

3-3: sass-embeddedとmodern APIの統合実装は適切です。

すべてのsass-loaderインスタンスを検出してsass-embeddedを設定するアプローチは完全で、依存関係も正しく宣言されています(sass-embedded@^1.83.0、sass-loader@^14.2.1)。deprecation警告の事前制御も適切に実装されています。

Likely an incorrect or invalid review comment.

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