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

app/template/smartphone のgit管理 #4713

Merged

Conversation

KentarouTakeda
Copy link
Contributor

@KentarouTakeda KentarouTakeda commented Oct 2, 2020

概要(Overview・Refs Issue)

  • app/template/smartphone 配下のファイルをgit管理出来るようにする
  • app/template/smartphone ディレクトリが初期状態より存在しているようにする

方針(Policy)

  • .gitignroe
    admin, default, plugin, user_data は管理除外されていないことより smartphone も同じ対応を行うべきと考えました。
  • .gitkeep
    EC CUBE 本体側に src/Eccube/Resource/template/smartphone/.gitkeep が存在するためカスタマイズ配下にも設置すべきと考えました。

スマホ向けにライトなカスタマイズを行おうとしている方としては MobileTemplatePathListener の実装を見つけずとも、ディレクトリの使い分けだけでテンプレート切り替えが出来ることをフォルダ構成より明示出来る点でより親切になるんじゃないかと思います。

実装に関する補足(Appendix)

テスト(Test)

  • app/template/smartphone ディレクトリ配下に -f オプション無しでファイルを追加できることを確認しました。

マイナーバージョン互換性保持のための制限事項チェックリスト

  • 既存機能の仕様変更
  • フックポイントの呼び出しタイミングの変更
  • フックポイントのパラメータの削除・データ型の変更
  • twigファイルに渡しているパラメータの削除・データ型の変更
  • Serviceクラスの公開関数の、引数の削除・データ型の変更
  • 入出力ファイル(CSVなど)のフォーマット変更

レビュワー確認項目

  • 動作確認
  • コードレビュー
  • E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
  • 互換性が保持されているか
  • セキュリティ上の問題がないか

@okazy okazy added the improvement 機能改善 label Oct 5, 2020
@okazy okazy added this to the 4.0.x milestone Oct 5, 2020
Copy link
Contributor

@okazy okazy left a comment

Choose a reason for hiding this comment

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

ありがとうございます!
最初から smartphone があった方が断然わかりやすいですね。
動作確認して問題ありませんでした。

@okazy okazy modified the milestones: 4.0.x, 4.0.6 Oct 20, 2020
@kiy0taka kiy0taka merged commit 632c8d0 into EC-CUBE:4.0 Oct 21, 2020
@okazy
Copy link
Contributor

okazy commented Oct 21, 2020

ありがとうございます!マージされました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants