-
Notifications
You must be signed in to change notification settings - Fork 32
Migrate to functions v2 #168
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: master
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.
I would revert all the "/v2" sort of migrations--this codebase is already on V2 which has been the default for a while.
As a design pattern I might make a params/config namespace where you've exported all your params (they're immutable, so you should be OK providing direct const access). Then when you're initializing globals, you should do so in an onInit callback, during which params can be safely accessed.
const emailDebug = params.defineBoolean("EMAIL_DEBUG"); | ||
const emailGroup = params.defineString("EMAIL_GROUP"); | ||
|
||
export function getGitHubToken(): string { |
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.
What is the purpose of these helpers? Is this a common enough need that we should take feedback for the Functions SDK?
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.
I just wanted them to be named so I didn't have to call params.defineString(string)
everywhere. Some of these are used only once so I'm not really saving any work in those cases.
5cf5fbc
to
a18c17f
Compare
No description provided.