pylint as part of build ?

Having trouble installing or compiling FreeCAD? Get help here.
mlampert
Posts: 1257
Joined: Fri Sep 16, 2016 9:28 pm

pylint as part of build ?

Postby mlampert » Sun Jun 30, 2019 10:46 pm

We've started using linters and cleaning up the Path python sources - and I was wondering if it was possible to have pylint installed on the build boxes so we could integrate a lint run into Path's unit tests?

I think it would be OK if we run it only on say linux builds if pylint is not available for Windows - or it's a pain to get installed.
User avatar
sgrogan
Posts: 5207
Joined: Wed Oct 22, 2014 5:02 pm

Re: pylint as part of build ?

Postby sgrogan » Mon Jul 01, 2019 9:29 pm

mlampert wrote:
Sun Jun 30, 2019 10:46 pm
I was wondering if it was possible to have pylint installed on the build boxes so we could integrate a lint run into Path's unit tests?
It should just require adding pylint and pylint3 @line 152 of https://github.com/FreeCAD/FreeCAD/blob ... s.yml#L152
I can make a pull request if you want, or you can do it in the commit set that starts using it.
If this is going to be a hard dependency for Path tests we will need to add it to the PPA, which is straight forward.

For the Win Travis builds I would need to add pylint and astroid to the libpack. Both are pure python so it should be straight forward.

For the OSX Travis builds I don't know, but they are disabled, so I wouldn't worry now. Maybe a comment in the travis.yml in case someone gets it working again in the future.

For the Conda based builds there is a Conda package available, it should be straight forward to add.

So the question is to make it a hard dependency or use it in some try/except block?
I can do most of it either way. If it becomes a hard dependency we need to document on the wiki and make an announcement for other Distros' package manager.
mlampert
Posts: 1257
Joined: Fri Sep 16, 2016 9:28 pm

Re: pylint as part of build ?

Postby mlampert » Tue Jul 02, 2019 12:22 am

sgrogan wrote:
Mon Jul 01, 2019 9:29 pm
So the question is to make it a hard dependency or use it in some try/except block?
It's probably OK to have it as a soft dependency and the unit test checks if it is installed or not. Linting should lead to the same results regardless of the platform it is being run on so if we can enable it for one of the builds we can automatically run it.
User avatar
sgrogan
Posts: 5207
Joined: Wed Oct 22, 2014 5:02 pm

Re: pylint as part of build ?

Postby sgrogan » Wed Jul 03, 2019 9:44 pm

mlampert wrote:
Tue Jul 02, 2019 12:22 am
It's probably OK to have it as a soft dependency and the unit test checks if it is installed or not. Linting should lead to the same results regardless of the platform it is being run on so if we can enable it for one of the builds we can automatically run it.
Sounds good.
How about we add this to the travis linux (py3 and py2 builds) Please tell me if you want me to do it.
I will work on adding the dependencies to the Win Libpack, so Win developers can use it locally, but I won't add it to the Libpack that Travis uses (thus saving resources).
mlampert
Posts: 1257
Joined: Fri Sep 16, 2016 9:28 pm

Re: pylint as part of build ?

Postby mlampert » Thu Jul 04, 2019 2:53 am

sgrogan wrote:
Wed Jul 03, 2019 9:44 pm
How about we add this to the travis linux (py3 and py2 builds) Please tell me if you want me to do it.
I will work on adding the dependencies to the Win Libpack, so Win developers can use it locally, but I won't add it to the Libpack that Travis uses (thus saving resources).
That sounds great. For py2 it would be pylint and for py3 it is pylint3 - given that py2 is EOL there's probably not much value in installing it. pylin3 also has more checks so it would be the better solution. I still have to clean up quite a bit of Path before it runs through without warnings. I can add it to the PR once I have it all cleaned up, I'll ping you once I get everything together.
looo
Posts: 2664
Joined: Mon Nov 11, 2013 5:29 pm

Re: pylint as part of build ?

Postby looo » Thu Jul 04, 2019 5:26 am

How about license? Pylint seems to be gpl3. So I guess the dependency must be optional?
User avatar
PrzemoF
Posts: 2610
Joined: Fri Jul 25, 2014 4:52 pm
Contact:

Re: pylint as part of build ?

Postby PrzemoF » Thu Jul 04, 2019 6:53 am

Another thread about code quality checker: https://forum.freecadweb.org/viewtopic.php?f=10&t=32602
reox
Posts: 595
Joined: Sat Aug 13, 2016 10:06 am

Re: pylint as part of build ?

Postby reox » Thu Jul 04, 2019 7:28 am

looo wrote:
Thu Jul 04, 2019 5:26 am
How about license? Pylint seems to be gpl3. So I guess the dependency must be optional?
IMO: the usage of a tool is a different thing than linking code. I would assume you are not putting pylint into FC but simply use it as a tool...
User avatar
saso
Posts: 1279
Joined: Fri May 16, 2014 1:14 pm
Contact:

Re: pylint as part of build ?

Postby saso » Thu Jul 04, 2019 8:00 am

Is flake8 that we are already running basically not the same thing ? https://travis-ci.org/FreeCAD/FreeCAD/jobs/554080767

Linters like this are IMO best used directly in developers IDE's where the code is checked as you write it. It would be best to agree on a common setup for the python code and since FEM developers are using it already from the beginning it is probably best to just follow their standard...
mlampert
Posts: 1257
Joined: Fri Sep 16, 2016 9:28 pm

Re: pylint as part of build ?

Postby mlampert » Thu Jul 04, 2019 6:36 pm

saso wrote:
Thu Jul 04, 2019 8:00 am
Is flake8 that we are already running basically not the same thing ? https://travis-ci.org/FreeCAD/FreeCAD/jobs/554080767
I wasn't aware of flake8 - will have to look into it.
Linters like this are IMO best used directly in developers IDE's where the code is checked as you write it.
which is why I want it to be part of the (at least one) build - not every programmer is running linters and as we grow it becomes harder to maintain a level of consistency. My goto solution for such issues to automate it and make sure developers (especially myself) don't "forget" to clean up after them ;)