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

[Mike Blumenkrantz]

On Wed, May 18, 2016 at 3:51 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Thu, 12 May 2016 17:20:49 +0200
> Miguel Angel Vico <mvicomoya at nvidia.com> wrote:
>
> > Thanks Derek.
> >
> > Inline.
> >
> > On Thu, 12 May 2016 08:54:26 -0500
> > Derek Foreman <derekf at osg.samsung.com> wrote:
> >
> > > On 11/05/16 10:53 AM, Miguel Angel Vico wrote:
> > > > Thanks, Yong.
> > > >
> > > > Inline.
> > > >
> > > > On Wed, 11 May 2016 09:45:15 -0500
> > > > Yong Bakos <junk at humanoriented.com> wrote:
> > > >
> > > >> On May 11, 2016, at 7:48 AM, Miguel A. Vico <mvicomoya at nvidia.com>
> > > >> wrote:
> > > >>>
> > > >>> No functional change. This patch only renames gl_renderer_create()
> > > >>> to gl_renderer_display_create(), which is something more
> > > >>> descriptive of what the function does.
> > > >>>
> > > >>> Signed-off-by: Miguel A Vico Moya <mvicomoya at nvidia.com>
> > > >>> Reviewed-by: James Jones <jajones at nvidia.com>
> > > >>
> > > >> Hi Miguel,
> > > >> When renaming, I suggest adjusting the indentation of subsequent
> > > >> arguments as well, to preserve alignment. Example noted inline
> > > >> below. I'm noticing this issue throughout this whole patch
> > > >> series.
> > > >
> > > > The thing is arguments aren't correctly aligned throughout all
> > > > weston code. I think the code-style rule of using hard-tabs instead
> > > > of spaces for indentation made things to be a bit messy, since
> > > > arguments are usually aligned using hard-tabs as well, but I think
> > > > spaces should be used instead for those cases in order to keep a
> > > > correct indentation regardless tab length settings of the editor.
> > >
> > > The style of this weston source code is to use hard tabs, 8 chars
> > > wide. It won't look right in an editor configured with any other tab
> > > width.
> > >
> > > I'm with Yong on this one - if you're going to rename a function, it's
> > > preferable to fix up the indenting at the same time for those call
> > > sites (even if they didn't match the style guidelines before).  This
> > > fix-up should be done with tabs for alignment.
> > >
> > > Otherwise trivial refactoring patches would rapidly make the code look
> > > terrible.
> >
> > Okay, sounds sane to me too. I'll send an updated patch.
> >
> > >
> > > For what it's worth, I personally prefer tabs for indenting and spaces
> > > for alignment -
> >
> > I prefer spaces for everything, but I can live with tabs for
> > indenting and spaces for alignment :-)
> >
> > > but that's not what this project uses, so it's not
> > > what I use on this project. :)
> >
> > I double-checked code-style rules here:
> >
> >   https://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing
> >
> > and wrt alignment it only states:
> >
> >   - when breaking lines with functions calls, the parameters are aligned
> >     with the opening parentheses;
> >
> > To me, that doesn't mean we shouldn't use spaces for alignment.
>
> Hi,
>
> as with all styles, there are exceptions.
>
> When breaking lines and aligning, it often does not happen exactly at a
> hard-tab boundary, so use spaces for the final adjustment.
>
> However, if any single argument to a function call cannot be reasonably
> fit by the alignment requirement without breaking the max line length,
> then (at least I have) you can forget the alignment to the opening
> parenthesis and just indent as far as you can and align there, using
> hard-tabs only.
>
> Sometimes it might be preferable to use shortcut variables, sometimes
> not. Rules can be bent if it is necessary for style consistency and
> readability. After all, the whole point of a style definition is to make
> things more readable to everyone on average, and almost always
> consistency in style helps that, so people should follow the project
> style rather than personal preferences.
>
> And yes, Weston does not rigorously follow the defined style to the
> point absolutely everywhere. Sometimes it is just better to land a v6
> of a patch than to go a few more review cycles to hone the style. Even
> reviewers can get tired of it, and we're usually short on review
> bandwidth anyway.
>

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.


>
>
> Thanks,
> pq
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160518/dcb1f815/attachment-0001.html>