determine if we should generate alerts for compiler warnings or just track the overall trend, also who is the owner

RESOLVED FIXED in Firefox 57

Status

Testing
Talos
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Trunk
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [PI:August])

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
We need a consistent path for sheriffing alerts that come up in perfherder.  For the compiler warnings there are concerns they are not consistent and nagging people about adding 3 warnings seems overkill.

I see a few options:
1) ensure we have tools/docs so developers can understand compiler warnings and file for everything
2) set a higher threshold for alerts (say >20), we still need docs, but it becomes more self explanatory when the number changes more extreme
3) do not alert at all for compiler warnings and just store the data for someone who cares about compiler warnings to track them.

In all cases, I need to know who is the person who can be the point of contact and effectively own the compiler warnings.  As sheriffs we take alerts and notify the responsible party, we are not subject matter experts which is why we require an owner.

I need:
* owner (s)
* decision on level of alerts
(Assignee)

Comment 1

5 months ago
:ehsan, :gps mentioned you would be the responsible party here, can you make the decisions here?
Flags: needinfo?(ehsan)
Whiteboard: [PI:July]

Comment 2

5 months ago
I'm lacking some context here.  For example I don't know what information is available in Perfherder so perhaps what I'm saying below won't make a lot of sense.  But I think there is value in filing bugs for newly introduced compiler warnings in non-third-party code if we can detect that, but beyond that nagging people, requiring owners for those bugs etc. is overkill, since the value of the fix to those bugs would differ on a case by case basis.  Not sure how easy this is, and how much of a time investment it is either, those aspects may indeed make it overkill.  There are other ways of dealing with warnings so not doing anything here is definitely not the end of the world IMO.
Flags: needinfo?(ehsan)
(Assignee)

Comment 3

5 months ago
:gps can you help :ehsan out here with what is sent to perfherder and what it means?  I assume you added the upload of data to perfherder.

Is there a way to not upload warnings for third-party changes?  I want to make this actionable without needing a lot of extra caveats.

If we decide not to notify patch authors of compiler warning changes, I would request that we disable alerts.
Flags: needinfo?(gps)

Comment 4

5 months ago
`mach build` parses compiler warnings out of build output. Currently, we associate a single integer count of compiler warnings against the build in Perfherder. We don't treat 3rd party projects specially. If we wanted, we could start reporting multiple warning counts. e.g. 3rd party vs non-3rd party.

Personally, I feel we should suppress warnings for 3rd party code that we have no intent on fixing within the build system itself. They won't get emitted and we won't have to deal with them in metrics land.
Flags: needinfo?(gps)
(Assignee)

Comment 5

4 months ago
following up from the original comment in the bug:
I need:
* owner (s)
* decision on level of alerts
** I see the mention of not generating alerts for 3rd party compiler warnings but there is not a clear path to make that happen.


I would like to get this resolved sooner than later as we continue to generate alerts for compiler warnings.  I would like to assert that if alerts are generated we need to file a bug and nag people, otherwise, lets track the data and turn alerts off.

we could set the alertThreshold much higher here:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py#2095

right now it is at 1.0, that is a 1% change; maybe we make it 5%, that would pick up large changes and probably many 3rd party updates as well- although not all of them.  Open to other ideas!

:igoldan, is this something you could experiment with a bit and help drive to resolution?
Flags: needinfo?(ionut.goldan)
Whiteboard: [PI:July] → [PI:August]
(Assignee)

Comment 6

4 months ago
Created attachment 8901891 [details] [diff] [review]
disable alerts for compiler warnings

enough time has gone by without clear ownership- since then a lot of confusion when annotating bugs with regressions/improvements regarding what the specific warnings are.
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Flags: needinfo?(ionut.goldan)
Attachment #8901891 - Flags: review?(gps)

Updated

4 months ago
Attachment #8901891 - Flags: review?(gps) → review+

Comment 7

4 months ago
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cff86dd3a1f5
disable compiler warnings. r=gps

Comment 8

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cff86dd3a1f5
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.