[PATCH weston 1/5] gl-renderer: Rename gl_renderer_create to gl_renderer_display_create

[Miguel Angel Vico]

Thank you all for your clarifications.

I'm about to send updated revisions of all patches. I made a rebase and
fixed all indentation/alignment issues across all patches.

Thanks.

On Wed, 18 May 2016 15:31:32 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
> 
> On 18 May 2016 at 15:25, Derek Foreman <derekf at osg.samsung.com> wrote:
> > On 18/05/16 08:41 AM, Mike Blumenkrantz wrote:  
> >> In fairness, we'd likely be less short on review bandwidth if the
> >> majority of that bandwidth was not in use to make/revise trivial
> >> criticisms such as whitespace usage and comment grammar which are
> >> guaranteed to be cleaned up in future patches; this leads to
> >> burnout on both the code-writing side as well as the reviewing
> >> side since everyone has become hyper attuned to the insignificant
> >> and non-functional minutiae of patches rather than focusing more
> >> on expediting the technical development of the protocol.  
> >
> > Fair points, though I'm not certain "will certainly get fixed up
> > later" is a given.  Certainly indenting and basic style is a
> > mechanical problem that could be tested pre-commit hooks, and there
> > should be no reason to bike shed that on the list at all.
> >
> > Grammar probably needs more serious consideration for protocol doc
> > than elsewhere due to its potential impact on compositor
> > implementors - but ever there probably not to the degree we're
> > seeing lately.
> >
> > Follow up commits that do nothing but change style and grammar can
> > make "git blame" less useful (when trying to figure out who would
> > best review a piece of code - not just "arrrrgh who wrote this
> > stupid bug") and provide churn for very little benefit, imho.  
> 
> Yeah, I agree. I get that the bikeshedding can be annoying; I do (for
> that reason, if no other) like tagging commits as 'RFC' or similar,
> which is effectively, 'please just check out the technical concept and
> don't worry about memory leaks or spelling mistakes right now'. But
> given that it's pretty trivial to fix up, and you're likely to have to
> rebase _anyway_, I don't see the harm in doing one round of review for
> clarity.
> 
> Generally, there's no need to send out a subsequent revision round
> just because someone has noted some typos - send it again if you need
> a resend anyway to get people to pick it up after a rebase, or if
> there have been notable changes, but you shouldn't be arriving at v17
> just because you have difficulty spelling.
> 
> Similarly, 'no, I disagree' is a reasonable response to someone
> bikeshedding your exact choice of variables or naming. Review is meant
> to be a discussion, not something you just have to unilaterally
> acqiuesce to.
> 
> > While we're drifting just slightly off topic here, I'm also
> > concerned about the basic usage of some of our tags:
> >
> > Reviewed-by:  indicates rigorous technical review *AND* a firm
> > conviction that the feature is important and should be included.
> >
> > Acked-by: Indicates a firm conviction that the feature is important
> > and should be included, but no rigorous review has taken place.
> >
> > Signed-off-by: Indicates an assumption of responsibility for the
> > code.
> >
> > I've seen a lot of Reviewed-by that I think is just "I looked at the
> > spelling and you're good to go".  I'm starting to find this
> > disconcerting.  
> 
> Yes, completely agreed. There's a huge gulf between 'I looked at this
> and nothing bad jumped out', versus 'I understand the implications
> with this and am prepared to put my name down in agreeance with it
> being a good idea'. I am pretty bad with review latency in general,
> but on the other hand I do at least give them a reasonably thorough
> look over ...
> 
> Cheers,
> Daniel


-- 
Miguel