-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat(config): variables #35350
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(config): variables #35350
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.
would be nice to have the refactor in separate PR, otherwise lgtm
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.
Create refactor PR first
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.
failed checks
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 was the reason for keeping variables and secrets "apply" function separate?
No reason :) |
Is there any place where applying secrets and applying variables aren't done at the same time? If there's never any separation of them, these functions should be combined. |
Needs some attention 🤒 |
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.
otherwise LGTM
🎉 This PR is included in version 41.23.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Changes
util/interpolator.ts
to house the repeated logic betweensecrets
andvariables
Context
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: