All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Purdie <richard.purdie@linuxfoundation.org>
To: Ed Bartosh <ed.bartosh@linux.intel.com>,
	bitbake-devel@lists.openembedded.org
Subject: Re: [PATCH 1/4] bitbake: main: fix bad-witespace pylint warnings
Date: Tue, 26 Apr 2016 11:44:17 +0100	[thread overview]
Message-ID: <1461667457.31320.160.camel@linuxfoundation.org> (raw)
In-Reply-To: <1461572190-17421-1-git-send-email-ed.bartosh@linux.intel.com>

This patchset triggered a lively discussion between a few of us about
whether we should or shouldn't make whitespace changes to the codebase.

There is one viewpoint which is that we should state we're aiming for
PEP8 and modify the codebase to match.

Unfortunately there are pieces of API and conventions we have which do
make sense for us and don't match PEP8. For example the use of "d" as
our datastore variable (its a short variable name) or the camelcase in
the datastore methods (getVarFlag) which happens to have worked really
well for the variable dependency analysis.

Personally, I also really don't like unnecessary changes in the history
so that git blame and code annotation get distorted to the whitespace
change commits. I probably spend more time doing that than most.

My general thought is that we should aim for something PEP8 like,
particularly around argument and keyword spacing. I'd prefer to do this
as we add new code or modify existing code and I have been doing that
in my own changes for a while to try and improve consistency.

That said, I personally don't get worked up about line length limits
and even the short variable names aren't that bad, *if* they're limited
in scope to a handful of lines or follow a codebase wide convention (fn
is filename, lf is lockfile, e is exception are the main bitbake ones
that come to mind).

So my feeling is we continue to try and improve the style but not go as
far as large whitespace changes just for the sake of it.

There are some sections which might be worth considering such as Ed's
changes here to the continuation and line too long pieces of main.py
specifically as it is currently a mess. I think the short variable
change may actually confuse some conventions more than fixing them
though.

Certainly I don't want to see a ton of "PEP8" style cosmetic patches
hitting the list.

Cheers,

Richard


  parent reply	other threads:[~2016-04-26 10:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-25  8:16 [PATCH 1/4] bitbake: main: fix bad-witespace pylint warnings Ed Bartosh
2016-04-25  8:16 ` [PATCH 2/4] bitbake: main: fix bad-continuation warnings Ed Bartosh
2016-04-25  8:16 ` [PATCH 3/4] bitake: main: fix line-too-long pytling warnings Ed Bartosh
2016-04-25  8:16 ` [PATCH 4/4] bitbake: main: fix invalid variable names Ed Bartosh
2016-04-26 10:44 ` Richard Purdie [this message]
2016-04-26 14:38   ` [PATCH 1/4] bitbake: main: fix bad-witespace pylint warnings Christopher Larson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1461667457.31320.160.camel@linuxfoundation.org \
    --to=richard.purdie@linuxfoundation.org \
    --cc=bitbake-devel@lists.openembedded.org \
    --cc=ed.bartosh@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.