Re: Code Review bot new reporting style

[Bastien Abadie]

The error levels are displayed at the top of a revision, in the Diff detail
> Lint section. You'll see a list of issues there, the first column
indicates their Severity.

Good point about replying to lint issues, I don't think Phabricator allows
developers to do that, as lint issues are transient.

Bastien Abadie

On Sat, Mar 28, 2020 at 2:20 AM Gerald Squelart 
wrote:

> On Friday, March 27, 2020 at 11:11:46 PM UTC+11, Bastien Abadie wrote:
> > The code review bot has been updated today and now publishes all issues
> > found on your patches as Phabricator lint results (Diff Detail section)
> > instead of inline comments.
> >
> > Here is a sample revision showcasing the new style.
> > <https://phabricator-dev.allizom.org/D1758> (on phabricator staging)
> >
> >
> > The main difference for developers is that lint results are tied to a
> > specific patch; so when you update your revision, the lint results are
> > automatically removed and only new ones are displayed (if any).
> >
> > Please be aware that any Error found by the bot must be fixed before
> > landing, or your patch will break the CI. Warnings can still be ignored
> and
> > should not break the build.
> >
> > The summary comment remains the same, and will always appear in the
> > revision to keep track of the revision evolution.
> >
> > Thanks a lot to all the developers
> > <https://bugzilla.mozilla.org/show_bug.cgi?id=1608339> who suggested
> this
> > styling change, we hope you’ll enjoy this enhancement.
> >
> > If you have any questions regarding this change or the code review bot,
> you
> > can reach us on Matrix #code-review-bot
> > <https://chat.mozilla.org/#/room/#code-review-bot:mozilla.org>.
> >
> >
> > Bastien Abadie
>
> It looks nice, thank you! Not seeing expired issues will be great.
>
> > ... Error must be fixed ... Warnings can be ignored ...
>
> Reviewbot used to prefix messages with "Warning" or "Error", but I don't
> see it in Lint messages. How can we make the distinction?
>
> Also Reviewbot comments could be replied to, but I don't see a way to do
> that with Lint messages, is that intended?
>
> Cheers,
> Gerald
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Code Review bot new reporting style

[Bastien Abadie]

The code review bot has been updated today and now publishes all issues
found on your patches as Phabricator lint results (Diff Detail section)
instead of inline comments.

Here is a sample revision showcasing the new style.
<https://phabricator-dev.allizom.org/D1758> (on phabricator staging)


The main difference for developers is that lint results are tied to a
specific patch; so when you update your revision, the lint results are
automatically removed and only new ones are displayed (if any).

Please be aware that any Error found by the bot must be fixed before
landing, or your patch will break the CI. Warnings can still be ignored and
should not break the build.

The summary comment remains the same, and will always appear in the
revision to keep track of the revision evolution.

Thanks a lot to all the developers
<https://bugzilla.mozilla.org/show_bug.cgi?id=1608339> who suggested this
styling change, we hope you’ll enjoy this enhancement.

If you have any questions regarding this change or the code review bot, you
can reach us on Matrix #code-review-bot
<https://chat.mozilla.org/#/room/#code-review-bot:mozilla.org>.


Bastien Abadie
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Update of the Code review bot: CI-breaking issues are now reported

[Bastien Abadie]

tl;dr: The Code Review Bot will now warn you about CI-breaking issues on
Phabricator

Hello,

We have just released a new version of the code review bot that changes
slightly how lint issues are reported on Phabricator:

   -

   Issues breaking the CI are now reported as Phabricator lint results.
   Those are unignorable, contrary to inline comments.
   We are highlighting them because, if landed, such patches would be
   backed-out by the sheriffs.
   They are displayed at the top of the revision, and in the patch. They
   also are automatically removed on new diffs.
   -

   Ignorable lint issues (warnings) and static analysis results are still
   reported as inline comments.


This will help you know when your patch has issues which would cause a
back-out.

You can view a demonstration of such an error report here:
https://phabricator.services.mozilla.com/D59862

Bastien & Marco
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Upcoming changes to hg.mozilla.org access

[Bastien Abadie]

On Saturday, November 2, 2019 at 12:03:25 AM UTC+1, somb...@gmail.com wrote:
> For mozilla-beta, mozilla-release, and esr... does lando know how to 
> land to these, or is it the case that landings on these branches are 
> done based on the approval flags by people other than the patch author?

I'm working on bringing the uplift request workflow to Phabricator and Lando.
When that work is done, you will be able to make an uplift request for any 
mozilla-central patch towards mozilla-beta, mozilla-release, and esr from Lando.

> I ask because if I create a branch based on the hg unified repo 
> "release" tag and then use `moz-phab` to create a review, I assume what 
> happens if I try and land with "lando" is that it will try and land the 
> commit against mozilla-central and it may succeed if the file hasn't 
> changed too much in central versus where it was on the release branch.

The workflow will also support diffs directly created on a repository that 
needs approval from Release Management: you will be able to use Lando to fill 
the uplift form and directly update your existing revision.

This work is still in early stages, and we are planning to iterate with Release 
Management and a few developers on the best workflow. I'll send an update to 
dev-platform once it's available.

Bastien

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Running source code analysis on Try

[Bastien Abadie]

Hello Nicholas,

First, i'm sorry the bot did not analyze your patch. I've asked a
Phabricator admin to restart the build, and it went fine
<https://phabricator.services.mozilla.com/harbormaster/build/89310/>.

I'm aware of the issue
<https://github.com/mozilla/release-services/issues/2106> you reported and
I am actively working on it. A fix should be in production in the next few
days.

I don't want to bore you with the details, but your build failed on the
first step for the code review. This is the only part we are hosting on
Heroku, and that does not scale easily. We are in the process of moving
that piece to GCP with CloudOps
<https://bugzilla.mozilla.org/show_bug.cgi?id=1536853>; things should be
better there.

Finally, as any distributed or complex system, an SLA of 100% is impossible
to achieve. If you encounter an issue later on, feel free to contact me
directly, or post on the Slack channel #phabricator

Cheers,

Bastien Abadie

On Thu, May 16, 2019 at 6:18 AM Nicholas Alexander 
wrote:

> Bastien, others,
>
> On Tue, May 7, 2019 at 9:38 AM Bastien Abadie  wrote:
>
>> TL;DR: We are leveraging the try infrastructure to perform source code
>> analysis (linters, static analyzers, etc). You can take advantage of this
>> to trigger other try jobs on Phabricator revisions.
>>
>
> What is the status of this work?  I want to take advantage of the new
> infrastructure to finally turn certain Android analysis tasks into proper
> lints (see Bug 1512487 [1]) but the "Plan 4 Source Code Analysis" build
> plan appears to both instantly fail and be opaque (see, for example [2]).
> I believe that "reviewbot" was very unhappy last night; perhaps that is
> related?
>
> Even with setbacks, I'm excited for these changes to make it easier to
> build analysis tasks!
> Nick
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1512487
> [2] https://phabricator.services.mozilla.com/harbormaster/build/89310/
>
>
>>
>> For more than a year, the Release Management & Quality team has been
>> running the Code review bot
>> <https://phabricator.services.mozilla.com/p/reviewbot/> using several
>> Mozilla in-house projects: Taskcluster <http://docs.taskcluster.net/> &
>> release-services <https://github.com/mozilla/release-services>.
>>
>> Our process was simple at first: for every new patch on Mozreview, then
>> Phabricator, we would run a few analyzers in a Taskcluster task. It worked
>> well for some time, but did not provide the flexibility nor the
>> performance
>> needed to support more analyzers (clang-format, Infer, Coverity, …)
>>
>> So we decided to experiment with a more scalable architecture, using
>> Treeherder/Try to run all necessary analyzers for your patches, in
>> parallel.
>>
>> This move will give us a lot of benefits:
>>
>>-
>>
>>The analyzers themselves are well defined and exposed in
>>mozilla-central, allowing us to use the exact same tools as our usual
>> CI
>>and the developers on their computers.
>>-
>>
>>Better overall performance and stability (from mercurial clones to
>>running analyzers safely in parallel)
>>-
>>
>>A standard analyzer output in JSON will allow us to easily add new
>>analyzers. These new implementations could even come from you!
>>
>>
>> Over the last few months, we have been building the different pieces
>> needed
>> to move the bot on the Try infrastructure, and today, I’m glad to announce
>> it’s running in production, on your revisions!
>>
>> Any new Diff on Phabricator should now have a build plan named Source Code
>> Analysis <https://phabricator.services.mozilla.com/harbormaster/plan/4/>.
>> Phabricator triggers a code review as soon as the diff is published,
>> automatically creating a new try job. You’ll get a “Treeherder Jobs” link
>> next to the build plan when it’s created by the bot, allowing you to check
>> on the analysis progress, and dive into the issues.
>>
>>
>> You will also be able to use Treeherder to add new jobs, effectively
>> making
>> it possible to run try tests for Phabricator revisions in just a few
>> clicks. On a Treeherder push, open its action menu on the right side
>> (right
>> after the Pin button). The "Add new jobs" options shows all the available
>> jobs grouped by platform and test suite, and you can click to select and
>> then submit. "Add new jobs (Search)" allows you to search in the job names
>> like mach try fuzzy and add them.
>>
>> If the patch contains any issues, they wi

Running source code analysis on Try

[Bastien Abadie]

TL;DR: We are leveraging the try infrastructure to perform source code
analysis (linters, static analyzers, etc). You can take advantage of this
to trigger other try jobs on Phabricator revisions.

For more than a year, the Release Management & Quality team has been
running the Code review bot
 using several
Mozilla in-house projects: Taskcluster  &
release-services .

Our process was simple at first: for every new patch on Mozreview, then
Phabricator, we would run a few analyzers in a Taskcluster task. It worked
well for some time, but did not provide the flexibility nor the performance
needed to support more analyzers (clang-format, Infer, Coverity, …)

So we decided to experiment with a more scalable architecture, using
Treeherder/Try to run all necessary analyzers for your patches, in parallel.

This move will give us a lot of benefits:

   -

   The analyzers themselves are well defined and exposed in
   mozilla-central, allowing us to use the exact same tools as our usual CI
   and the developers on their computers.
   -

   Better overall performance and stability (from mercurial clones to
   running analyzers safely in parallel)
   -

   A standard analyzer output in JSON will allow us to easily add new
   analyzers. These new implementations could even come from you!


Over the last few months, we have been building the different pieces needed
to move the bot on the Try infrastructure, and today, I’m glad to announce
it’s running in production, on your revisions!

Any new Diff on Phabricator should now have a build plan named Source Code
Analysis .
Phabricator triggers a code review as soon as the diff is published,
automatically creating a new try job. You’ll get a “Treeherder Jobs” link
next to the build plan when it’s created by the bot, allowing you to check
on the analysis progress, and dive into the issues.


You will also be able to use Treeherder to add new jobs, effectively making
it possible to run try tests for Phabricator revisions in just a few
clicks. On a Treeherder push, open its action menu on the right side (right
after the Pin button). The "Add new jobs" options shows all the available
jobs grouped by platform and test suite, and you can click to select and
then submit. "Add new jobs (Search)" allows you to search in the job names
like mach try fuzzy and add them.

If the patch contains any issues, they will be reported as before, through
inline comments. You can now restart a build if it fails (click on the
Build, there is a “Restart Build” link in the right sidebar). Please note
that secure revisions are not supported for now, but we are actively
working on that (a build failure will be shown for secure revisions for the
time being).

We would like to thank the different people and teams involved in this
effort: Andrew from the Automation team; Tom, Rail, Chris and Rok from
Release Engineering; Dustin and Peter from the Taskcluster team; Steven,
glob and David from the Engineering Workflow team.

If you have any questions regarding this move or the code review bot, feel
free to contact us by mail or on IRC #static-analyzers.

Bastien, on the behalf of the Release Management and Quality team.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform