On Thu, Apr 26, 2018 at 01:20:28PM +0300, Pekka Paalanen wrote: > On Thu, 26 Apr 2018 11:46:54 +0200 > Drew DeVault <[email protected]> wrote: > > > On 2018-04-26 10:49 AM, Pekka Paalanen wrote: > > > when someone merges this patch, please do add a commit message > > > explaining why these events are added. See > > > https://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing#n21 > > > for guidance on what to write. > > > > > > Even if it seems obvious to everyone right now, it's not obvious after > > > 5 years. I don't think the one line summary explains it. > > > > > > For example, we had a long discussion about having just one event > > > instead of two, what that one event would mean, and yet we ended up > > > with two separate events (which I think is for the better). I would > > > expect the commit message to explain why we have two events instead of > > > one, since having one was the original and intuitive proposal. > > > > > > Acked-by: Pekka Paalanen <[email protected]> > > > > > > I would give R-b if the commit message was there. The protocol spec > > > text looks good to me. > > > > I respectfully disagree. The commit message should pertain only to the > > final approach, and historical information and timely commentary belongs > > on the mailing list. 5 years from now, should it prove confusing, > > looking up the mailing list posts will not be considerably more > > difficult than looking up the commit message. > > A commit must always document the "why exactly this change is being > made", and here it is completely missing, even for the final approach.
There should be a commit message explaining the change, yes. Sorry for missing that when reviewing. Jonas > > > Thanks, > pq > _______________________________________________ > wayland-devel mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
