Thanks for this feedback Chris, my comments in-line.:
On Thu, 2014-07-24 at 15:00 -0700, Christopher J. Morrone wrote:
Some notes for improvements needed:
The URL under the git clone line on the wiki page needs fixing. It is
specific to you, Richard.
Next, I would like to see this wiki page on the opensfs wiki instead
(or in addition to) the Intel wiki.
Sound good to me.
The information about how to build and use lustre-static-analysis
be added to the lustre-static-analysis code tree.
Agreed. I personally favor input from users who use the tool to identify
unused code in Lustre to drive this process.
The _new_ code is GPLv2, but the part(s) taken from clang/llvm are under
the University of Illinois/NCSA Open Source License, which I think is
basically just the BSD.
Why did you choose GPLv2 when the tool you based it on is BSDish?
I have considered the mixing of Licenses. We did the work under
contract. The contract specified GPL2 for code. GPL2 and Clang BSD
license are compatible. I sided with the contract.
We need a bit more of documentation of exactly what the two plugins
My suggestion here is that we keep the build and usage instructions in a
single readily available format until more feedback arrives. At that
point we can consider a suitable README in the code or similar.
I would like to see lustre-static-analysis given a bit fuller of a
system. Ideally it would use autotools. At a minimum it would have a
top-level make file.
As part of Removal of Dead Code, this plugin proved valuable -- but it
must be said that with the conclusion of that project, we cannot commit
to on-going maintenance. The license, of course, allows anyone to
modify, distribute and provide that role.
If there is general interest in this tool, I guess the best place is to
voice that here and at the TWG?
Next, it seems to me like the method of invoking the tool is a little
strange. I think I would prefer to have a script that behaves like a
compiler (it calls clang for analysis, and then passes the full command
line to gcc for the real compilation). Right now we have a psuedo-make
So I would like the lustre build commands to look like:
$ sh autogen.sh
$ ./configure CC=my_fine_lustre_clang_wrapper
That just seems a little more obvious to me then the way it is currently
invoked. Granted, I might be missing something.
Probably not. Our project goal was to delete unused code from Lustre
. We developed this tool enough to get us to the goal. There is more
unused code, and I think further use of this tool will drive the next
Intel High Performance Data Division