1857421 - Add a glinter for redundant values in `send_in_pings`

Status: RESOLVED WONTFIX

(Data Platform and Tools :: Glean: SDK, enhancement)

Description

[Alessio Placitelli [:Dexter]]

We should likely report as a linting problem if the send_in_pings only contains one value and that value is the default ping for that metric type.

Comment 1

[Jan-Erik Rediger [:janerik]]

Talking about this briefly during triage we weren't sure why we want to do that.
Being explicit about this seems to be a useful documentation feature. Any change to our defaults would require communication anyway and so would need to be done carefully, regardless of whether send_in_pings is set explicitly or not.

Thus we're leaning towards a WONTFIX here.
Any pushback, :dexter?

Comment 2

[Alessio Placitelli [:Dexter]]

(In reply to Jan-Erik Rediger [:janerik] from comment #1)

Being explicit about this seems to be a useful documentation feature. Any change to our defaults would require communication anyway and so would need to be done carefully, regardless of whether send_in_pings is set explicitly or not.

Isn't the documentation part covered by the Glean Dictionary? I can imagine the Glean Dictionary report the explicit list of pings something is sent in regardless of it being explicitly set or not.

The use-case I had in mind was events: folks tend to add

send_in_ping:
  - events

to the YAML for events, which is redundant and doesn't really convey much information that isn't already conveyed via the Glean Dictionary. It makes the YAML declaration feel more verbose than it needs to be, and with folks copy pasting around, this grows over time.

Thus we're leaning towards a WONTFIX here.
Any pushback, :dexter?

No, no pushback, it's your decision :) Please make sure to explicitly document why you feel this is ok the way it is and why reducing verbosity for this case is bad, so that I can point people to it. Thanks!

Comment 3

[Jan-Erik Rediger [:janerik]]

While editing the file, adding or modifying metrics, people rarely have the dictionary open on the site, thus having inline documentation of intention is still useful.

Comment 4

[Alessio Placitelli [:Dexter]]

(In reply to Jan-Erik Rediger [:janerik] from comment #3)

While editing the file, adding or modifying metrics, people rarely have the dictionary open on the site, thus having inline documentation of intention is still useful.

Should then be made a mandatory field?

Comment 5

[Jan-Erik Rediger [:janerik]]

(In reply to Alessio Placitelli [:Dexter] from comment #4)

Should then be made a mandatory field?

That would require a whole bunch of migration, which we're not equipped to do at this point. Thus we're still going for WONTFIX, it's not important enough to address it.