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

renovate minorやpatchアップデートををまとめないように #437

Merged
merged 2 commits into from
Sep 2, 2021

Conversation

youchann
Copy link
Contributor

これはjust ideaですので、議論大歓迎。

#435 のようにスタックしがち。
やるには腰が重くなりがちなので、PRをまとめないように設定を変更する。

@youchann youchann requested a review from a team as a code owner August 27, 2021 07:53
@youchann youchann requested review from ryokosuge and removed request for a team August 27, 2021 07:53
@netlify
Copy link

netlify bot commented Aug 27, 2021

✔️ Deploy Preview for ingred-ui ready!

🔨 Explore the source changes: 5e999f8

🔍 Inspect the deploy log: https://app.netlify.com/sites/ingred-ui/deploys/613026c493fccb0007cd44b6

😎 Browse the preview: https://deploy-preview-437--ingred-ui.netlify.app

@youchann youchann self-assigned this Aug 27, 2021
@kohashi
Copy link
Contributor

kohashi commented Aug 30, 2021

なんらかのグループ化、または patch バージョンだけはまとめるとかしないとPR溢れて大変になるのでは? 🤔
という気もしますので、難しいとこですね…。

#435 のコケた理由は snapshot の不一致で、 styled-components の生成する乱数が変わった感じだからなのかな?
てかこの class="sc-乱数" の 乱数はなんかの種があるのかな。
ちょっとわかってないけど、理由がstyled-components なら renovate分割してもあんま関係なさそうな(問題の切り分けがしやすい!というのはありそうか)

@ryokosuge
Copy link
Contributor

なんらかのグループ化、または patch バージョンだけはまとめるとかしないとPR溢れて大変になるのでは? 🤔

これは自分も同じ気持ちしましたが、1つずつ対応しないで一気に(1つずつのPRを)確認することを考えると別々のPRで1つずつ確認できる方が確かにやりやすそうだなって思いました!
一気にというかまとまった時間内で全部を確認するって感じですね

@youchann
Copy link
Contributor Author

youchann commented Sep 1, 2021

#435 くらいまとまってしまうと、各々のリリースノート追うのに時間がかかるので「とにかくCI通してリリースする」のが最優先になってしまうのですよね(自分はそうだった)。

あるべき状態は、各packageのアップデートをキャッチアップしてそれを現リポジトリに反映させることだと考えておりまして。(それと似た思想があってgithubネイティブなdependabotはPRをまとめる機能がない)

なので分けてしまおうかな。という背景もあります!!

@youchann
Copy link
Contributor Author

youchann commented Sep 1, 2021

とはいえpatchまで入れるのか?には議論の余地があります。(管理画面のほうはpatch を追わないようにしてちょうどよく回ってきている)

@kohashi
Copy link
Contributor

kohashi commented Sep 1, 2021

なるほどこのへん。
https://github.com/voyagegroup/fluct_ms/blob/master/renovate.json#L13-L14

Patch Version 追う/追わないみたいなの難しいですねー。
たいていは「一部のバグ修正・セキュリティ上の修正」なのでむしろ積極的に取り込みたいような気もするし、それはそれで「知らない間にバグ混入」とかありそう。

"patch": { "automerge": true },  とかで「CIでチェック通ったらマージ」とするのもありえそう。
いまググったらリクルート傘下のスタディサプリが同様にしている
https://tech.recruit-mp.co.jp/front-end/post-21350/

@kohashi
Copy link
Contributor

kohashi commented Sep 1, 2021

とりあえず「minorは分割する」の部分は賛成です!

@youchann
Copy link
Contributor Author

youchann commented Sep 1, 2021

ああ、patchは自動でマージ、minorは分割。みたいな方針で良さそう。

@youchann
Copy link
Contributor Author

youchann commented Sep 1, 2021

↑で一旦どうでしょう....?

@youchann
Copy link
Contributor Author

youchann commented Sep 2, 2021

@kohashi @ryokosuge ↑どうですかねー:thinking:

@ryokosuge
Copy link
Contributor

@youchann

patchは自動でマージ、minorは分割。みたいな方針で良さそう。

いいと思います!賛成です!

@kohashi
Copy link
Contributor

kohashi commented Sep 2, 2021

👍
まぁこれも運用してみて「うーん」ってなったら再調整だし、とりあえずの方針としては :LGTM: です!

@youchann
Copy link
Contributor Author

youchann commented Sep 2, 2021

了解ですー、ではその設定にしてマージしておきます!

@youchann youchann merged commit 0171b18 into master Sep 2, 2021
@youchann youchann deleted the tweak-renovate-config-split-non-major branch September 2, 2021 01:22
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