Re: Intent to Enable: Automated Static Analysis feedback in MozReview

It's being investigated: https://github.com/mozilla-releng/services/issues/707 On Nov 7, 2017 19:05, "Boris Zbarsky" wrote: On 10/4/17 10:32 AM, Jan Keromnes wrote: > We've already disabled this "no defects" comment, and are currently > deploying the fix to production, so the

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

On 10/4/17 10:32 AM, Jan Keromnes wrote: We've already disabled this "no defects" comment, and are currently deploying the fix to production, so the bot should stop sending them soon. It looks like the "no defects" comment is back? See https://bugzilla.mozilla.org/show_bug.cgi?id=1149250#c30

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

On Tue, Oct 17, 2017 at 4:00 PM wrote: > Those things can be encoded as regexps, but they span across programming > languages (XUL, XHTML, HTML, XBL, DTD, JS, C++). > To create a new regex-based linter, simply add a file like this:

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

On Tue, Oct 17, 2017 at 9:58 PM, wrote: > This is awesome! As an engineer who has to work with C++ until we advance > Rust bindings, I always feel terrible when my reviewers spend their > precious time identifying simple C++ errors in my code. > > > Seeing the

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

This is awesome! As an engineer who has to work with C++ until we advance Rust bindings, I always feel terrible when my reviewers spend their precious time identifying simple C++ errors in my code. Seeing the advancements in static analysis for C++, rustfmt and eslint for JS, I'm wondering if

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

Thanks glandium! Yes, reporting defects that are expanded from macros generates a lot of noise, and it's not always an issue we can fix (e.g. when the macro comes from third-party code). So we've decided to stop reporting these, and the fix was merged in time for tomorrow's push to production.

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

On Tue, Oct 17, 2017 at 10:38:09AM +0200, Jean-Yves Avenard wrote: > Hi > > Do you have ways to prevent running this code on some external > frameworks? > > Whenever we modify a gtest this causes hundreds of errors about not > using a default constructor … (which shouldn’t be an error anyway)

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

Hi Do you have ways to prevent running this code on some external frameworks? Whenever we modify a gtest this causes hundreds of errors about not using a default constructor … (which shouldn’t be an error anyway) > On 9 Oct 2017, at 4:33 pm, Jan Keromnes wrote: > > > C/C++

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

Thanks Patrick. C/C++ was our top priority because code defects are very costly, but we'd love to make our static analysis bot support additional languages, so we're looking into integrating with mozlint [0] (the `./mach lint` wrapper around eslint, flake8 and wptlint), and I think we should

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

This sounds awesome Jan. Are there plans to have something similar for JS code linting (with ESLint)? On Wed, Oct 4, 2017 at 7:14 PM, Ehsan Akhgari wrote: > On 10/04/2017 03:17 AM, Jan Keromnes wrote: > >> TL;DR -- We wrote a static analysis bot for MozReview

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

On 10/04/2017 03:17 AM, Jan Keromnes wrote: TL;DR -- We wrote a static analysis bot for MozReview ("clangbot") and it's about to complain about any patches that would introduce new C/C++ code defects to Firefox. This is fantastic to see, thank you for making it happen!

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

On 10/4/17 10:32 AM, Jan Keromnes wrote: We've already disabled this "no defects" comment, and are currently deploying the fix to production, so the bot should stop sending them soon. Great, thank you! No need to apologize, by the way. Bugmail noise happens; thank you for moving on it

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

> Not sure this is a bug, but... > > I got bugmail today from this bug saying that a patch (not mine; just a bug I was cced on) didn't have any problems. Can we consider having the bot add bug noise only when there _is_ a problem? Indeed this is problematic, and we're very sorry about this. We

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

On 10/4/17 3:17 AM, Jan Keromnes wrote: Please report any bugs with the bot here: https://bit.ly/2y9N9Vx Not sure this is a bug, but... I got bugmail today from this bug saying that a patch (not mine; just a bug I was cced on) didn't have any problems. Can we consider having the bot add

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

We do have tooling which analyze changes after landing (coverity is probably the most visible but we have tooling based on Clang tidy too), we do report some of the issues but it takes time (and we only report defects which seem critical or easy to fix like dead code). Now, because of the

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

> But it's not analyzing patches that are not using MozReview. Will those patches be analyzed after landing? Indeed, our bot doesn't run on patches that are attached to Bugzilla (Splinter reviews) or directly landed. However, I believe that the Mozilla checkers we use are also run on Try, and

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

On 04/10/2017 09:17, Jan Keromnes wrote: You can also run them on your own code with: ./mach static-analysis check path/to/file.cpp Sounds awesome! I tried this locally to see what it would say about a random(ish) file in the tree, but it ended with the message: 0:42.99 Could not find

Re: Intent to Enable: Automated Static Analysis feedback in MozReview

This sounds interesting! But it's not analyzing patches that are not using MozReview. Will those patches be analyzed after landing? Nick On Wed, Oct 4, 2017 at 6:17 PM, Jan Keromnes wrote: > TL;DR -- We wrote a static analysis bot for MozReview ("clangbot") and it's > about

Intent to Enable: Automated Static Analysis feedback in MozReview

TL;DR -- We wrote a static analysis bot for MozReview ("clangbot") and it's about to complain about any patches that would introduce new C/C++ code defects to Firefox. Please report any bugs with the bot here: https://bit.ly/2y9N9Vx In an effort to improve the quality of Firefox, we want to