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

プルリクエストマージ時のデフォルトコミットメッセージは何にするのが良いか #56

Open
Hiroshiba opened this issue Aug 16, 2024 · 14 comments
Labels
要議論 実行する前に議論が必要そうなもの

Comments

@Hiroshiba
Copy link
Member

質問の内容

プルリクエストをマージする時、デフォルトでどういうメッセージにするかいくつか選べたりします。

image

でも結構いろんな条件でいろんな課題が発生するので、何にするべきか薄く意見を募集したいです。

課題

  • そのブランチのコミットメッセージを入れる場合
    • [skip ci]と書かれたメッセージをマージした時に、mainブランチのCIをスキップされてしまう
      • skip ciを禁止にする手もありそうだけど、そういう設定はどうやらなさそう?
  • プルリクエストのdescriptionをメッセージにする場合
    • description内に<detail>などがあってものすごく長くなってる場合にメッセージがとんでもない量になる
      • base64の画像が直接入ってる場合とかもありそう(?)
    • 中身に http://github.com/hoge/fuga/issues/#123みたいなコメントが入ってるコードがあると、そこへのリンクが張られてしまうかも(要検証)
      • 別に問題ではないだろうけど、なんかこう・・・
  • プルリクエストのタイトルだけ入れる場合
    • ぶっちゃけこれが一番無難だけど、本当にいいんだろうかという気持ちが多少ある

その他

完全に「自転車置き場の屋根の色」なんですが、何か考え出すと楽しいので考えちゃってます。
ご意見募集中です 🙏

@Hiroshiba Hiroshiba added the 要議論 実行する前に議論が必要そうなもの label Aug 16, 2024
@Hiroshiba
Copy link
Member Author

個人的な意見としては、プルリクエストマージ時にそのプルリクエストの番号が書かれて、github上だとすぐにそのリンクへ飛べるのと、問題を考えるのがめんどくさいので、まあコミットメッセージはタイトルだけでいいかなと思い始めました。

ただ自分は1行目以外本当に全くコミットメッセージを見ないので、2行目以降のコミットメッセージを結構見る方がいらっしゃれば意見聞きたいです 🙏

@qryxip
Copy link
Member

qryxip commented Aug 18, 2024

一応私はコミットメッセージを見る人の一人だと思います。

コミットメッセージの本文があることの利点の一つとして、git logを漫然と見ながら下方向(/)と上方向(?)に検索をかけれることがあると思います。git-log(1)以外でも、例えばNeovimのNeogitではlogを折り畳みつつ本文部分の検索ができたりします。もちろんキーボードから一時も指を離すことなく。このスタイルであれば10,000行のPR descriptionであってもさほどの影響は無いと思います。
(インスパイア元であるEmacsのMagitも同じことができる…と思ったのですがよく覚えていない)

image

<Neogit起動のキーバインド>ll/segfault<C-j>

image

C-mまたはReturn

image

あとPRのdescriptionをマージメッセージにしているっぽい有名リポジトリを調べてみました。マージ時にわざわざPRの「結論」をまとめ直してメッセージを書いているらしきところもそこそこ目にしました。
(ちなみにですが、有名リポジトリを片っ端から調べてみると"Create a merge commit"方式のリポジトリがかなり多かったです。"Squash and merge"に匹敵するどころがこちらの方がメジャーでは?と思えるほど)

PRのdescriptionをメッセージにしているっぽいもの

PRのdescriptionをベースにしつつ、たまに手書きしているっぽいもの

タイトルのみのもの

割愛。ただし結構あって、Pythonもそうなはず。


まあGitHubとの相性の悪いGit自体の機能はいくつもありますし、正直コミットメッセージもその一つだと思います。実際ここにいる多くの人はコミットメッセージを全く見ないとは思うので、判断は任せます。

@Hiroshiba
Copy link
Member Author

Hiroshiba commented Aug 18, 2024

気になって自分も調べてみました。
正直descriptionでも良さそうに思いました!
もし定型文がある場合はメッセージを短くしたほうが良さそう。(今のとこほぼないので問題ない)

タイトルのみ
https://github.com/vuejs/core
https://github.com/microsoft/TypeScript
https://github.com/electron/electron // 各commitメッセージを束ねたのをメッセージにしてることもある

descriptionベース
https://github.com/elastic/elasticsearch
https://github.com/redis/redis
https://github.com/flutter/flutter // チェックリストとかは除外

merge commitベース(ここより上は全部squash merge)
https://github.com/kubernetes/kubernetes

その他
https://github.com/apache/kafka // JIRAチケットあるものはマージメッセージがない
https://github.com/django/django // ?
https://github.com/openssl/openssl // リベースマージ?

@qryxip
Copy link
Member

qryxip commented Sep 12, 2024

descriptionベースですが、コミットメッセージ本文はMarkdownとみなされないため```の中に'#'や'@'が入ってたりするとリンクされてしまうようです。microsoft/onnxruntimeでも結構発生しているみたい。
例: microsoft/onnxruntime@3e93403

この問題を避けるならマージ時に「PRの結論」を丁寧に書くのが多分ベストで、次点でタイトルのみ、という感じになるかなと…

@Hiroshiba
Copy link
Member Author

Hiroshiba commented Sep 12, 2024

なーーーーるほどです・・・ 😇

とりあえずタイトルのみにしようか迷ってます。思考回路こんな感じです。

  • commit message含むと問題が起こる
  • description含んでも問題が起こる
  • 問題が起こってほしくない
  • マージ時に手動で結論を書けば問題は起こらないけど、はあまり現実的ではない
    • 書いてもらった結論をマージ時に自動的にメッセージに含める、などのbot機能はアリだと思うけど、時間がかかる
  • とりあえず問題が起こらないように、一旦タイトルのみにするのはどうか

「タイトルのみにする」か「skip ciされちゃっても良いから全コミットメッセージ含む」かだと、タイトルのみのが実害でなくてマシ・・・・・・・なのかなぁ・・・。

追記)あ! メンテナ次第では「手動でマージ時に書く」もあり!!!

@qryxip
Copy link
Member

qryxip commented Sep 12, 2024

追記)あ! メンテナ次第では「手動でマージ時に書く」もあり!!!

コミットメッセージ本文は基本空白になるけど、マージボタンを押す人がなんか書いたりするのは止めない、という感じでしょうか?

@Hiroshiba
Copy link
Member Author

あっすみません!
メンテナ側で「コメントを書いてからマージするようにする」と取り決めを決めておけば、運用でカバーできるという意図でした!

コアの主体なメンテナは @qryxip さんという認識(と @Hiroshiba@y-chan もだけど)なので、例えばですがコアは毎回 @qryxip さんがコメントを書いてもマージボタンを押す、ということにするとか。
で、自分とかは緊急時にしかマージしない(&タイトルのみマージ)みたいな。
(もしやるなら、今の運用だとレビュワーの方も2つapproveが集まればマージできる形になっているので、そこをどうするかも考慮ポイントかも。とりあえずそのまま運用でもほとんど問題なさそう。)

@qryxip
Copy link
Member

qryxip commented Sep 13, 2024

なるほどです。ちょっと試しに、 VOICEVOX/voicevox_core#831 のマージで「PRの結論」を書いてみました。

image

@Hiroshiba
Copy link
Member Author

おー!!!
見ていて思ったのですが、結論はわりとプルリクエストを作った段階ではなく、マージされる段階で完成されるので、プルリクエストを作った方にdescriptionに書いていただいてもダメかもですね。
(自動化しづらいということ)

全然関係ないけど、変更内容とディスカッションからこれくらい的を射た要約を作れるLLMが現れるととてもうれしそう。まだまだ先だろうけど。

@qryxip
Copy link
Member

qryxip commented Sep 19, 2024

第二弾。自分の中での整理になりますし結構いいですねこれ。
image

@Hiroshiba
Copy link
Member Author

Hiroshiba commented Sep 19, 2024

おお、なるほどです!!
ではとりあえずVOICEVOX COREだけ、一旦様子見としてデフォルトコミットメッセージをタイトルのみにしてみますか!
他のリポジトリは一旦そのまま・・・?で良いかなぁ。

2024/09/19 23:57追記 VOICEVOX COREをtitleのみにしてみました!
また何かあれば何でもコメントいただければ!!

@qryxip
Copy link
Member

qryxip commented Sep 29, 2024

ヒホさんが作った VOICEVOX/voicevox_core#840 についても「結論」のメッセージを書いてみました(中身はヒホさんが書いたdescriptionほぼそのままです)。

ただまあ、Git的にはauthorとcommitterという概念があるのですが、GitHub的にはcommitterという概念は無いのでヒホさんがこのテキストを打ち込んだように見える…

image

@Hiroshiba
Copy link
Member Author

ただまあ、Git的にはauthorとcommitterという概念があるのですが、GitHub的にはcommitterという概念は無いのでヒホさんがこのテキストを打ち込んだように見える…

あ、たしかに!!
最初に「(メンテナによる要約)」とか書けば問題ないかも?
あるいは貢献者ガイドライン等に「メンテナによる要約をコミットメッセージとする運用をしています」と書いておくとかでも良さそう。

まあ何も書かずにもうしばらく運用してみるのでも良さそう!

@Hiroshiba
Copy link
Member Author

(報告だけ)

エディタの方も、プルリグタイトルだけが入るようにしておきました!

スナップショットを更新するためのコミットメッセージがあるんですが、もしかしたらそれが反応してそうだったので、ついでにエディタ側もタイトルのみにしちゃおうかなと!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
要議論 実行する前に議論が必要そうなもの
Projects
None yet
Development

No branches or pull requests

2 participants