-
Notifications
You must be signed in to change notification settings - Fork 59
Warn on Missing Environment Variables and Continue Build #22
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
Conversation
…turn missing variables and log warnings, improving build resilience.
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.
Pull Request Overview
This PR updates the build process to warn on missing environment variables instead of failing, allowing builds to complete even when vars aren’t set.
- Replaced exception-throwing for missing env vars with a warning log in webpack configs.
- Modified
createEnvDefinePluginto return bothenvKeysandmissingVarsinstead of throwing. - Added an initial log statement when fetching environment variables.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| workspaces/wso2-platform/wso2-platform-extension/webpack.config.js | Replaced try/catch with destructuring of envKeys and missingVars; added warnings |
| workspaces/mi/mi-extension/webpack.config.js | Same environment-handling update as WSO2 extension |
| workspaces/ballerina/ballerina-extension/webpack.config.js | Same environment-handling update as WSO2 extension |
| common/scripts/env-webpack-helper.js | Changed createEnvDefinePlugin to return missingVars instead of throwing |
Comments suppressed due to low confidence (1)
common/scripts/env-webpack-helper.js:26
- [nitpick] The JSDoc return type is generic; consider specifying a structured return type like
@returns {{envKeys: Object, missingVars: string[]}}for better clarity.
* @returns {Object} Object containing envKeys for webpack.DefinePlugin and missingVars array
| console.warn(error.message); | ||
| console.warn('Continuing build with empty environment variables...'); | ||
| envKeys = {}; | ||
| console.log("Fetching values for environment variables..."); |
Copilot
AI
Jul 7, 2025
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.
[nitpick] Using console.log for a routine build message can clutter output; consider using console.info or a dedicated logger so warnings remain prominent.
| console.log("Fetching values for environment variables..."); | |
| console.info("Fetching values for environment variables..."); |
| console.warn('Continuing build with empty environment variables...'); | ||
| envKeys = {}; | ||
| console.log("Fetching values for environment variables..."); | ||
| const { envKeys, missingVars } = createEnvDefinePlugin(env); |
Copilot
AI
Jul 7, 2025
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.
The same environment variable handling logic is duplicated across multiple webpack configs; consider extracting this into a shared utility to reduce duplication.
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.
This log should be in the webpack, otherwise the rush build log won't show the output. That's why all the logs are duplicated in the webpack config files.
Purpose
This pull request updates the build process to show warnings for missing environment variables instead of causing the build to fail. This allows processes like pull request checks, which lack configured environment variables, to complete successfully. The warning logs will still help developers identify any missing variables.