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

feat: use combined DRIFT/api index product list #5038

Closed
wants to merge 1 commit into from

Conversation

chingor13
Copy link
Contributor

This implementation uses the combined product list which is maintained by snippet-bot (from #5014)

Fixes #5036

constructor(allowedApiShortnames: Set<string>) {
this.allowedApiShortnames = allowedApiShortnames;
}
static async build(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not add these arguments to the constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not a huge fan of this class reaching out and doing stuff implicitly given an octokit or GCS connection/bucket.

The class really only cares about the allowed shortnames and the schema. Therefore, I opted to put the API calls into a helper method and pass only the data necessary for the validator into the class.

@chingor13
Copy link
Contributor Author

Closing in favor of #5037

@chingor13 chingor13 closed this Apr 25, 2023
@chingor13 chingor13 deleted the rml-product-db branch September 18, 2023 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[repo-metadata-lint] does not support non-gRPC APIs
2 participants