-
Notifications
You must be signed in to change notification settings - Fork 349
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
Generate modules metadata #232
Generate modules metadata #232
Conversation
This PR contains changes to the CI definitions. Reviews from @Azure/bicep-admins are needed. |
@@ -0,0 +1,40 @@ | |||
async function getModulesMetadata({ require, core }) { |
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.
Mind adding a JSDoc comment to annotate the types of parameters?
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.
Sure, updated here:
20d412d
Thanks!
* @param {string} dir | ||
*/ | ||
function getSubdirNames(dir) { | ||
const fs = require("fs"); |
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.
Should fs
remain a parameter and be passed from the caller?
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.
Updated here:
20d412d
Thanks!
workflow_call: | ||
secrets: | ||
# Secrets must be passed from the caller workflow explicitly. | ||
CLIENT_ID: | ||
required: true | ||
TENANT_ID: | ||
required: true | ||
SUBSCRIPTION_ID: | ||
required: true | ||
workflow_dispatch: | ||
|
||
permissions: | ||
id-token: write | ||
contents: read |
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.
Are the secrets needed? If not, we should remove them.
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.
Yes, I'll be sending out a follow up PR to login to azure and write the modules metadata to storage account container. Let me know if you want me to remove it here and include it in other PR.
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.
No, in that case we can leave them as is.
Changes in the PR include:
Verified the changes in dogfood via:
bhsubra#1