Issue777

Title tests: run selection of clang-tidy checks
Priority wish Status resolved
Superseder Nosy List florian, jendrik, malte, silvan
Assigned To jendrik Keywords
Optional summary

Created on 2018-04-12.15:36:07 by jendrik, last changed by jendrik.

Messages
msg7079 (view) Author: jendrik Date: 2018-04-25.14:44:26
I moved the new info to a new page and fixed the typos.
msg7078 (view) Author: malte Date: 2018-04-25.12:08:39
On second thought, I can't find time for this right now, and I can't add an
unbounded number of items to the TODO list, so this will have to do. Two small
things I noticed that I suggest changing:

- The new text on http://www.fast-downward.org/ForDevelopers/CodingConventions
says "We recommend using Ubuntu 16.04 or 18.04 for Fast Downward development
since these versions are also used by the core developers and they allow
installing all dependencies except VAL and Uncrustify 0.61 via the package
manager." but that's not true: other dependencies not installed via the package
manager include CPLEX, py.test and pyflakes.

- The bitbucket-pipelines.yml file (in the comments) says "Setup" (noun) a few
times where it should be "Set up" (verb).

Longer term, it would be nice to have a clearer documentation of supported OS
versions and dependencies, in one place, talking separately about user
dependencies and developer dependencies. But I can't time for this at the moment.
msg7077 (view) Author: malte Date: 2018-04-25.11:54:33
I had a look, but I don't think this fits well under coding conventions. There
was some old related information there already, but I think that should perhaps
have its primary location somewhere else too, perhaps with a link from the
coding conventions page. I'll try to get this started.
msg7076 (view) Author: jendrik Date: 2018-04-25.07:39:34
I merged the code and added some infos to http://www.fast-downward.org/ForDevelopers/CodingConventions .
msg7071 (view) Author: malte Date: 2018-04-24.11:30:11
I saw no strong objections on the list, so I think you can go ahead with this.
As you do, please add something to the developer documentation that lists our
supported operating systems and operating system versions for developers.
msg7051 (view) Author: malte Date: 2018-04-13.11:36:17
Is a meeting the right thing for this? I would write an email to downward-dev.
msg7050 (view) Author: jendrik Date: 2018-04-13.11:27:34
OK, I'll add it to the meeting agenda.
msg7049 (view) Author: malte Date: 2018-04-13.11:02:12
OK, but please let's get buy-in from everybody before merging. For example for
me it means that I will no longer be able run the tests on my university
computer, as I'm running Ubuntu 17.04, which does not seem to have the
appropriate version.

If you're right and 16.04 has the package, then that means that there are
exactly two Ubuntu versions right now that people could use: 16.04 and 17.10.
This does seem quite limiting; after all, my version is not even a year old.
(You also mentioned 18.04, but that isn't even released yet.)
msg7048 (view) Author: jendrik Date: 2018-04-13.10:39:42
I understand your concern. However, obtaining the binaries looks quite simple for 
all major platforms (including MacOS and Windows): 
http://releases.llvm.org/download.html has prebuilt-binaries for clang and clang-
tidy (both are in a single archive for each system).
msg7047 (view) Author: malte Date: 2018-04-13.09:25:30
I am just worried we are increasing the barrier of entry to Fast Downward
development. If we want to say that Fast Downward developers must use Ubuntu
16.04+, that should be communicated clearly and agreed between everyone.
msg7046 (view) Author: jendrik Date: 2018-04-13.09:15:43
I think it's easy to set up on Ubuntu machines. The two required packages 
clang-5.0 and clang-tidy-5.0 are available in all supported Ubuntu versions 
since 16.04 (xenial, artful and bionic).

Setting this up on the grid and our Windows machine could be more difficult, 
but we have never run any style checks there before, since the style is checked 
by the Linux buildbot anyway.
msg7045 (view) Author: malte Date: 2018-04-13.09:08:36
Well, I think that definitely needs buy-in from everybody. I guess it would help
if we could first figure out how difficult this is to set up. If we can easily
set this up on all buildbots and on the grid, then I suppose it cannot be too
bad on our other machines.
msg7044 (view) Author: jendrik Date: 2018-04-13.09:01:49
Yes, but I think that's a good thing. Otherwise, tests might pass locally and 
fail remotely. Especially, if we ever specify checks with wildcards like 
"performance-*". Then different versions might use different sets of tests.
msg7043 (view) Author: malte Date: 2018-04-13.08:47:36
Looks good to me in principle, but does it mean that everyone has to install
exactly the same clang-tidy version (5.0)?
msg7042 (view) Author: jendrik Date: 2018-04-13.00:02:42
The integration worked well, I think. I made a pull request at 
https://bitbucket.org/jendrikseipp/downward/pull-requests/82 .

I selected only three clang-tidy checks to keep the diff small (performance-
for-range-copy, performance-implicit-cast-in-loop and performance-inefficient-
vector-operation). We can add more checks later. I think automatically finding  
suboptimal code like in the patch is a nice feature to have.
msg7040 (view) Author: jendrik Date: 2018-04-12.15:36:07
See https://clang.llvm.org/extra/clang-tidy/ for info about clang-tidy. Some 
quick tests give the impression that this tool could be useful for us. I'd like 
to see how difficult it is to integrate it into our testing infrastructure.
History
Date User Action Args
2018-04-25 14:44:26jendriksetmessages: + msg7079
2018-04-25 12:08:39maltesetmessages: + msg7078
2018-04-25 11:54:33maltesetmessages: + msg7077
2018-04-25 07:39:34jendriksetstatus: reviewing -> resolved
messages: + msg7076
2018-04-24 11:30:11maltesetmessages: + msg7071
2018-04-16 17:08:07silvansetnosy: + silvan
2018-04-13 16:32:36floriansetnosy: + florian
2018-04-13 11:36:17maltesetmessages: + msg7051
2018-04-13 11:27:34jendriksetmessages: + msg7050
2018-04-13 11:02:12maltesetmessages: + msg7049
2018-04-13 10:39:42jendriksetmessages: + msg7048
2018-04-13 09:25:30maltesetmessages: + msg7047
2018-04-13 09:15:43jendriksetmessages: + msg7046
2018-04-13 09:08:36maltesetmessages: + msg7045
2018-04-13 09:01:49jendriksetmessages: + msg7044
2018-04-13 08:47:36maltesetmessages: + msg7043
2018-04-13 00:02:42jendriksetstatus: in-progress -> reviewing
messages: + msg7042
2018-04-12 15:36:07jendrikcreate