[PATCHv6 wayland-protocols] Add name event to xdg-output

Jonas Ã…dahl jadahl at gmail.com
Thu Apr 26 11:29:10 UTC 2018


On Thu, Apr 26, 2018 at 01:20:28PM +0300, Pekka Paalanen wrote:
> On Thu, 26 Apr 2018 11:46:54 +0200
> Drew DeVault <sir at cmpwn.com> 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 <pekka.paalanen at collabora.co.uk>
> > > 
> > > 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
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel



More information about the wayland-devel mailing list