-
Notifications
You must be signed in to change notification settings - Fork 7
sample-envの変更 #26
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
base: master
Are you sure you want to change the base?
sample-envの変更 #26
Conversation
ktogo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
ktogo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PRは機能ごとに最小限の単位で切り出してください。
これには下記の利点があります:
- ✔️ PRの粒度を小さくすることでレビュアーがレビューしやすくなる
- ✔️ 1PR=1機能だと、機能ごとにMerge/Revertが可能になる
逆に1PRに複数機能を突っ込んでしまうと、下記のデメリットがあります:
- ❌ 全機能がPR通過するまでMergeされない
- ❌ なにか不具合があったときは機能すべてがRevertされる
なので、どんなケースでも原則としてPRは最小単位、1機能1PRにしましょう。
ktogo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
まだ機能が混在してるので、最小単位にPRの分割をお願いします!
(どのプロジェクトにも共通することなので、原則いかなるPRも最小単位で作成お願いします!)
| WP_ROOT=/var/www/html | ||
| WP_URL=http://localhost | ||
| WP_VERSION=4.8.2 | ||
| WP_VERSION=4.9.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ああごめん…書き方悪かったかな、
これもまた別PRでお願いします!
どっちも依存関係のない修正 (=別々の機能) なので、
こういうものは基本的に個別のPRにしましょう。
というのも、仮に18行目の修正をRevertしなきゃならなくなったとき、
こっちの修正も一緒にRevertされることになっちゃうからです。
|
あ、あとPRのタイトルは「何を変えたか」ではなく「どう変更したか」にお願いします。 今回のケースなら
みたいな。 |
coreを4.9.8にバージョンアップ
初期インストールプラグイン設定を追記