-
Notifications
You must be signed in to change notification settings - Fork 476
refactor(plugins/checks): migrate to v2 API #3541
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, one small nit
js/plugins/checks/src/index.ts
Outdated
return checksEvaluators(googleAuth, metrics, await projectId); | ||
}, | ||
list: async () => { | ||
return [checksEvaluators(googleAuth, metrics, await projectId).__action]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm are we definitely meant to use __action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I'm not too sure. It did seem to work, but I'll look into this.
const googleAuth = inititializeAuth(options?.googleAuthOptions); | ||
|
||
const projectId = options?.projectId || (await googleAuth.getProjectId()); | ||
const projectId = options?.projectId || googleAuth.getProjectId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this no longer need to be awaited?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still is awaited on line 69, 72, and 75
Edit: now 68 and 71
Resolves #3444
migration guide: https://docs.google.com/document/d/1s1YKwhqdrGN44IG7lSAwltxM3Bqh9p6jTcri8SNaBWk/edit?usp=sharing
Checklist (if applicable):