Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v2 00/12] default runtime test case for python packages v2
Date: Tue, 6 Nov 2018 08:56:56 +0100	[thread overview]
Message-ID: <20181106085656.425efe64@windsurf> (raw)
In-Reply-To: <5be0f4fb15b71_1ffa2af2b3fab998208b9@ultri5.mail>

Hello Ricardo,

On Mon, 05 Nov 2018 23:57:15 -0200, Ricardo Martincoski wrote:

> >  - Use an integer for the Python version variable, rather than a
> >    string, there is really no need for this to be a string I believe ?  
> 
> You are correct. There is no need for a string. I will use an integer.

On the other hand, this python_version = 2/3 is only one line, and it
saves only one line in the config fragment, BR2_PACKAGE_PYTHON=y or
BR2_PACKAGE_PYTHON3=y.

So we replace one fairly explicit line:

	BR2_PACKAGE_PYTHON=y

by just another line, which requires going to the base class to
understand what is does:

	python_version = 2

I.e, I am wondering if this refactoring is really useful in the end ?

> >  - Alternatively, you could derive the version of the Python
> >    interpreter to use from the child class name. Maybe this is too
> >    "implicit" and a bit tricky, but I wanted to mention this
> >    possibility. I.e, in the base class, you use
> >    self.__class__.__name__, and it gives you the actual name of the
> >    class that is instantiated. You can then check if the string
> >    contains Python2 or Python3, and decide which interpreter to use
> >    according to this. I am not saying I absolutely want this, I'm just
> >    offering this as an alternative solution.  
> 
> For comparison only, I created a patch on top of the wip v3:
> https://gitlab.com/RicardoMartincoski/buildroot/commit/c824c5dfe9c1b0953088c9a818575287fe984fc4
> 
> I guess/hope that using the explicit python_version is easier to understand for
> a newcomer.
> Maybe a matter of taste.

Yeah, I don't have a solid opinion on this. Perhaps the explicit
solution is more readable/clearer.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  reply	other threads:[~2018-11-06  7:56 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-16  0:42 [Buildroot] [PATCH 0/7] default runtime test case for python packages Ricardo Martincoski
2018-10-16  0:42 ` [Buildroot] [PATCH 1/7] support/testing: create default " Ricardo Martincoski
2018-10-22  7:55   ` Thomas Petazzoni
2018-10-23  3:15     ` Ricardo Martincoski
2018-10-16  0:42 ` [Buildroot] [PATCH 2/7] support/testing: use default test_run for python-autobahn Ricardo Martincoski
2018-10-16  0:42 ` [Buildroot] [PATCH 3/7] support/testing: use default test_run for python-cryptography Ricardo Martincoski
2018-10-16  0:42 ` [Buildroot] [PATCH 4/7] support/testing: use default test_run for python-incremental Ricardo Martincoski
2018-10-16  0:42 ` [Buildroot] [PATCH 5/7] support/testing: use default test_run for python-twisted Ricardo Martincoski
2018-10-16  0:42 ` [Buildroot] [PATCH 6/7] support/testing: use default test_run for python-txaio Ricardo Martincoski
2018-10-16  0:42 ` [Buildroot] [PATCH 7/7] support/testing: use default test_run for python-txtorcon Ricardo Martincoski
2018-11-02  4:12 ` [Buildroot] [PATCH v2 00/12] default runtime test case for python packages v2 Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 01/12] support/testing: use helper class in IPython test Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 02/12] support/testing: use helper class in Python test Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 03/12] support/testing: create intermediate class per Python version Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 04/12] support/testing: create default test case for python packages Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 05/12] support/testing: use TestPythonPackageBase for python-autobahn Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 06/12] support/testing: use TestPythonPackageBase for python-cryptography Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 07/12] support/testing: use TestPythonPackageBase for python-incremental Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 08/12] support/testing: use TestPythonPackageBase for python-twisted Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 09/12] support/testing: use TestPythonPackageBase for python-txaio Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 10/12] support/testing: use TestPythonPackageBase for python-txtorcon Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 11/12] support/testing: rename python* test cases Ricardo Martincoski
2018-11-02  4:12   ` [Buildroot] [PATCH v2 12/12] testing: add python-crossbar tests Ricardo Martincoski
2018-11-04 10:40   ` [Buildroot] [PATCH v2 00/12] default runtime test case for python packages v2 Thomas Petazzoni
2018-11-04 22:42     ` Ricardo Martincoski
2018-11-05  8:15       ` Thomas Petazzoni
2018-11-06  1:57         ` Ricardo Martincoski
2018-11-06  7:56           ` Thomas Petazzoni [this message]
2018-11-10  2:15             ` Ricardo Martincoski
2018-11-10  2:16   ` [Buildroot] [PATCH v3 0/8] default runtime test case for python packages Ricardo Martincoski
2018-11-10  2:16     ` [Buildroot] [PATCH v3 1/8] support/testing: create default " Ricardo Martincoski
2018-11-10  2:16     ` [Buildroot] [PATCH v3 2/8] support/testing: use TestPythonPackageBase for python-autobahn Ricardo Martincoski
2018-11-10  2:16     ` [Buildroot] [PATCH v3 3/8] support/testing: use TestPythonPackageBase for python-cryptography Ricardo Martincoski
2018-11-10  2:16     ` [Buildroot] [PATCH v3 4/8] support/testing: use TestPythonPackageBase for python-incremental Ricardo Martincoski
2018-11-10  2:16     ` [Buildroot] [PATCH v3 5/8] support/testing: use TestPythonPackageBase for python-twisted Ricardo Martincoski
2018-11-10  2:16     ` [Buildroot] [PATCH v3 6/8] support/testing: use TestPythonPackageBase for python-txaio Ricardo Martincoski
2018-11-10  2:16     ` [Buildroot] [PATCH v3 7/8] support/testing: use TestPythonPackageBase for python-txtorcon Ricardo Martincoski
2018-11-10  2:16     ` [Buildroot] [PATCH v3 8/8] testing: add python-crossbar tests Ricardo Martincoski
2018-11-13 19:57     ` [Buildroot] [PATCH v3 0/8] default runtime test case for python packages Thomas Petazzoni

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=20181106085656.425efe64@windsurf \
    --to=thomas.petazzoni@bootlin.com \
    --cc=buildroot@busybox.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox