On Wed, May 18, 2016 at 3:51 AM Pekka Paalanen <ppaala...@gmail.com> wrote:
> On Thu, 12 May 2016 17:20:49 +0200 > Miguel Angel Vico <mvicom...@nvidia.com> wrote: > > > Thanks Derek. > > > > Inline. > > > > On Thu, 12 May 2016 08:54:26 -0500 > > Derek Foreman <der...@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 <j...@humanoriented.com> wrote: > > > > > > > >> On May 11, 2016, at 7:48 AM, Miguel A. Vico <mvicom...@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 <mvicom...@nvidia.com> > > > >>> Reviewed-by: James Jones <jajo...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel >
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel