Sure, I think that I was the one to specifically say that I didn't agree that "running more tools was better". Sorry, I should have said more on that to explain why I felt that way.
Shouldn't projects consider the cost to each new tool that gets added the build chain? It seems undesirable to me for a project to run findbugs and coverity and errorprone and whatever else just because there is some chance that one might find something the other doesn't at the additional cost to build time (i don't what that might be - maybe not an issue?) and ongoing maintenance. In my mind there should be some added value for each new thing we add to the build chain. I don't know that the added value needs to be significant (that would be nice), but it should at least be explainable in a definitive way (e.g. "we need to add findbugs because it covers X, Y, Z and errorprone does not"). I think that I'd want that kind of explanation if someone moved to add a second tool after we'd chosen a first before I would jump on-board with such a change. To continue with the issue of tool selection while I think I can agree that one tool may or may not find "everything", right now, we have no tools in place for this at all. It is a green field. If we should introduce a tool it should be one that gets us the most bang for the buck and that would be one that handles multiple languages. We would focus on getting that particular tool into play first and then go through the process of turning on the various checks we want in that tool and, as we have multiple languages, we'd want to do that for each language we support. Once all that is done, then we would determine if there are things that aren't covered and whether or not additional tools are helpful or redundant. Adding stuff (i.e. "tools" but also features or new dependencies) just because it is available or because it provides a small incremental value usually doesn't feel right to me. We've haphazardly done things like this in the past and it's always ended up burning us which is why I'm advocating for a slow and logical approach to making this happen. For example, sometime back, we added an appveyor build to try to ensure the project built on windows. But after a while we came to realize that it was false failing on a fairly regular basis. The failed builds basically put red "X"s on all the PRs even though they were fine which made it hard to quickly figure out what was truly passing and what wasn't. We were dealing with all this extra effort just so that if someone on windows pulled the code they could build TinkerPop if they wanted. We learned that having appveyor in the mix was a small incremental value that wasn't really worth the effort expended since we generally found that no one was really building on windows in the first place. I can't even recall the last complaint about that issue. Anyway, I hope that explains where I'm coming from a bit. On Tue, Apr 3, 2018 at 11:24 AM, Roman Leventov <leventov...@gmail.com> wrote: > Could somebody explain how adding a multi-language tool makes adding a > single-language tool undesirable, given one doesn't strictly cover another? > > On Tue, 3 Apr 2018, 16:36 Stephen Mallette, <spmalle...@gmail.com> wrote: > > > Reviving this thread a bit - a quick summary of where things were left: > > > > 1. no one seems specifically against adding a tool of this kind (though > > Florian went so far as to be positive on doing so) > > 2. if we did add a tool it was a good suggestion to get the tool selected > > first and then slowly enable different checks. > > 3. having multi-language support in the tool was brought up as a possible > > requirement in tool selection > > > > the more i've thought about it the more that I think my preference for > > putting code analysis in place would involve using a tool that provided > > multi-language support. Therefore, something like sonarqube or coverity > > would be more appealing to me than the original JVM only solution that > was > > proposed. Is that basically the representative feeling on this for > > everyone? > > > > > > > > On Wed, Mar 21, 2018 at 10:03 AM, Robert Dale <robd...@gmail.com> wrote: > > > > > It has multi-language support - > > > https://community.synopsys.com/s/question/0D53400003WcsSiCAJ/what- > > > languages-are-supported-by-synopsys-static-analysis-coverity > > > > > > Robert Dale > > > > > > On Wed, Mar 21, 2018 at 10:01 AM, Robert Dale <robd...@gmail.com> > wrote: > > > > > > > Another option might be to use Coverity - > > https://scan.coverity.com/faq > > > - > > > > and run it in Travis CI > > > > > > > > Robert Dale > > > > > > > > On Wed, Mar 21, 2018 at 9:43 AM, Stephen Mallette < > > spmalle...@gmail.com> > > > > wrote: > > > > > > > >> > I think any proposed changes would be separate PRs and reviewed > on > > a > > > >> case-by-case > > > >> basis. > > > >> > > > >> yeah - i think if we want something like this the goal should first > be > > > to > > > >> decide on a tool (i can't say that i agree with Roman that "running > > more > > > >> tools is always better" btw). note i'm still saying, "if we want > > > something > > > >> like this" because while i'm not against it I'd like to be convinced > > > that > > > >> we need to worry about this now at this stage of TinkerPop's > > > development. > > > >> i'd have been more inclined to deal with this early on in > TinkerPop's > > > >> development rather than now. And, yes, also agree that i would like > to > > > see > > > >> the tool in first and then each introduction of "rule" be a separate > > > >> discussion and PR. > > > >> > > > >> > As for making it the default compiler, I think this would be a > > > profile, > > > >> disabled by default, at best at least until a time it can be > massaged > > to > > > >> where it works for this project. > > > >> > > > >> yeah....that might be a smart way to go > > > >> > > > >> > whether other static analysis tools would find most of the > problems > > > >> that > > > >> such a compiler can find and also work with other languages than > Java. > > > >> > > > >> very good thought. while the bulk of our code is java, we are far > > from a > > > >> single language code base. if there is one tool that can give us > some > > > nice > > > >> clean analysis across the entire code base, perhaps that's the best > > way > > > to > > > >> go (again, "if we want something like this"). > > > >> > > > >> > > > >> > > > >> > > > >> On Tue, Mar 20, 2018 at 8:41 AM, Florian Hockmann < > > > f...@florian-hockmann.de > > > >> > > > > >> wrote: > > > >> > > > >> > In general, I am +1 on adding a static analysis tool to our tool > > > chain. > > > >> > I just wonder whether a Java compiler is the best choice for us > > right > > > >> > now or whether other static analysis tools would find most of the > > > >> > problems that such a compiler can find and also work with other > > > >> > languages than Java. SonarQube[1] for example looks pretty nice. > It > > > also > > > >> > supports Python, C#, and JavaScript and we can probably just > > integrate > > > >> > it via GitHub to generate reports for the release branches and all > > PRs > > > >> > where it would then add issues in the form of inline comments. > > > >> > > > > >> > [1] https://www.sonarqube.org/ > > > >> > > > > >> > Am 20.03.2018 um 03:42 schrieb Robert Dale: > > > >> > > Some changes look good others look wrong evident by the failed > > > >> builds. I > > > >> > > think any proposed changes would be separate PRs and reviewed > on a > > > >> > > case-by-case basis. As for making it the default compiler, I > > think > > > >> this > > > >> > > would be a profile, disabled by default, at best at least until > a > > > >> time it > > > >> > > can be massaged to where it works for this project. > > > >> > > > > > >> > > On Mon, Mar 19, 2018 at 14:59 Roman Leventov < > > leventov...@gmail.com > > > > > > > >> > wrote: > > > >> > > > > > >> > >> See https://github.com/apache/tinkerpop/pull/819. > > > >> > >> > > > >> > >> 1) Are there any objections to using error-prone compiler > instead > > > of > > > >> > >> OpenJDK's javac compiler? > > > >> > >> 2) Stephen raised the question, that Tinkerpop project may > better > > > use > > > >> > >> another static analysis tool instead of error-prone. I have to > > > answer > > > >> > that > > > >> > >> no static analysis tool covers 100% of the errors found by any > > > other > > > >> > tool, > > > >> > >> so running more tools is always better. > > > >> > >> 3) Stephen raised the question, what checks should be enabled. > I > > > >> believe > > > >> > >> this is out of scope of the PR ( > > > >> > >> https://github.com/apache/tinkerpop/pull/819), > > > >> > >> because it doesn't (and doesn't intent to) enable any more > > checks > > > >> > beyond > > > >> > >> the default (= minimal) set of error patterns. More checks may > or > > > may > > > >> > not > > > >> > >> be enabled in later PRs and that could be discussed separately. > > > >> > >> > > > >> > > > > >> > > > > >> > > > > >> > > > > > > > > > > > > > >
