Mon, Sep 25, 2017 at 04:46:44PM CEST, nogahf(a)mellanox.com wrote:
> -----Original Message-----
> From: Jan Tluka [mailto:jtluka@redhat.com]
> Sent: Monday, September 25, 2017 5:26 PM
> To: Nogah Frankel <nogahf(a)mellanox.com>
> Cc: lnst-developers(a)lists.fedorahosted.org; mlxsw <mlxsw(a)mellanox.com>;
> Yotam Gigi <yotamg(a)mellanox.com>
> Subject: Re: [PATCH LNST 01/11] common: add a consts file for mrouter port
> related consts
>
> Mon, Sep 25, 2017 at 03:40:23PM CEST, nogahf(a)mellanox.com wrote:
> >Add a Consts file to Common that holds classes that have only constants
> >values and no methods.
> >Add there a class for L2 multicast-router port related consts.
> >
> >Signed-off-by: Nogah Frankel <nogahf(a)mellanox.com>
> >Reviewed-by: Yotam Gigi <yotamg(a)mellanox.com>
> >---
> > lnst/Common/Consts.py | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> > create mode 100644 lnst/Common/Consts.py
> >
> >diff --git a/lnst/Common/Consts.py b/lnst/Common/Consts.py
> >new file mode 100644
> >index 0000000..bdcff0b
> >--- /dev/null
> >+++ b/lnst/Common/Consts.py
> >@@ -0,0 +1,16 @@
> >+"""
> >+General perpuses constants file
> >+
> >+Copyright 2016-2017 Mellanox Technologies. All rights reserved.
> >+Licensed under the GNU General Public License, version 2 as
> >+published by the Free Software Foundation; see COPYING for details.
> >+"""
> >+
> >+__autor__ = """
> >+nogahf(a)mellanox.com (Nogah Frankel)
> >+"""
> >+
> >+class MCAST_ROUTER_PORT:
> >+ FIXED_OFF = 0
> >+ LEARNING = 1
> >+ FIXED_ON = 2
>
> Hi Nogah,
>
> I see that you're about to use this in a recipe. If so I think it should be
> placed under lnst/RecipeCommon/ directory rather than lnst/Common.
>
> Additionally the Consts.py sounds too general to me. Is this file
> intended also for other stuff than switchdev testing?
>
> Regards,
> Jan
Hi Jan
This file should contain a lot of consts classes, for multiple purposes.
This first class is recipe only, but other classes might not be. (Actually,
we have a code in writing right now that have a consts class that is in use
in some BridgeTool function).
I agree that this file is quite general, and in the future it should
probably become a package with several files under it , but I think that we
should wait until we have some more classes in there before deciding how to
break it apart.
Thanks
Nogah
Ok, if you're going to use it also in other code than recipe, I'm ok
with this.
-Jan