Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1833393 - New lint: check that all referenced pings are known #584

Merged
merged 1 commit into from May 22, 2023

Conversation

badboy
Copy link
Member

@badboy badboy commented May 16, 2023

This introduces a new category of checks: those that run across both pings and metrics and can thus apply cross-checks.

Todo:

  • Fix the types

glean_parser/lint.py Outdated Show resolved Hide resolved
tests/test_lint.py Show resolved Hide resolved
tests/test_lint.py Show resolved Hide resolved
@badboy badboy force-pushed the unknown-pings branch 2 times, most recently from fd7f39e to 5a83ebe Compare May 17, 2023 12:48
@badboy badboy marked this pull request as ready for review May 17, 2023 12:48
@badboy badboy force-pushed the unknown-pings branch 2 times, most recently from 8e4841f to 939d00c Compare May 17, 2023 12:50
@badboy badboy requested a review from chutten May 17, 2023 12:50
Copy link
Contributor

@chutten chutten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+wc

...though it occurs to me that now I'm going to be causing myself some grief in FOG since the organization of the GeneratedFile directives has us parsing the metrics yamls and pings yamls separately.

Oh well. Problem for future-chutten.

glean_parser/lint.py Show resolved Hide resolved
@badboy
Copy link
Member Author

badboy commented May 17, 2023

r+wc

...though it occurs to me that now I'm going to be causing myself some grief in FOG since the organization of the GeneratedFile directives has us parsing the metrics yamls and pings yamls separately.

Oh well. Problem for future-chutten.

Yeah, while implementing that I had the same thought.

@chutten
Copy link
Contributor

chutten commented May 17, 2023

r+wc
...though it occurs to me that now I'm going to be causing myself some grief in FOG since the organization of the GeneratedFile directives has us parsing the metrics yamls and pings yamls separately.
Oh well. Problem for future-chutten.

Yeah, while implementing that I had the same thought.

It'll be a good forcing function to complete the work Nika started in https://bugzilla.mozilla.org/show_bug.cgi?id=1686265 to reduce the number of times we're parsing these yamls.

This introduces a new category of checks: those that run across both
pings and metrics and can thus apply cross-checks.
@badboy badboy enabled auto-merge (rebase) May 22, 2023 12:14
@badboy badboy merged commit 31ff187 into main May 22, 2023
9 checks passed
@badboy badboy deleted the unknown-pings branch May 22, 2023 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants