-
Notifications
You must be signed in to change notification settings - Fork 0
fix workflow #2
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
fix workflow #2
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -185,9 +185,13 @@ function isInstalledAsDependency() { | |||||||||||||||||
| return cwd.includes('node_modules'); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Check if running in non-interactive environment (like npm install) | ||||||||||||||||||
| // Check if running in non-interactive environment (like npm install or CI) | ||||||||||||||||||
| function isNonInteractive() { | ||||||||||||||||||
| return !process.stdin.isTTY || process.env.CI === 'true' || process.env.NPM_CONFIG_INTERACTIVE === 'false'; | ||||||||||||||||||
| return !process.stdin.isTTY || | ||||||||||||||||||
| process.env.CI === 'true' || | ||||||||||||||||||
| process.env.GITHUB_ACTIONS === 'true' || | ||||||||||||||||||
| process.env.NPM_CONFIG_INTERACTIVE === 'false' || | ||||||||||||||||||
| process.env.CI === '1'; | ||||||||||||||||||
|
Comment on lines
+190
to
+194
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic for detecting a non-interactive environment can be simplified. The
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Get domain name from vnext.config.json | ||||||||||||||||||
|
|
@@ -232,18 +236,34 @@ async function setup() { | |||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| console.log('🚀 vNext Template Setup'); | ||||||||||||||||||
| console.log('=======================\n'); | ||||||||||||||||||
|
|
||||||||||||||||||
| // Check if domain folder from vnext.config.json exists | ||||||||||||||||||
| const domainFromConfig = getDomainFromConfig(); | ||||||||||||||||||
| if (domainFromConfig && isDomainFolderConfigured()) { | ||||||||||||||||||
| // In CI/non-interactive mode, skip silently | ||||||||||||||||||
| if (isNonInteractive()) { | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
| // In interactive mode, show message | ||||||||||||||||||
| console.log('🚀 vNext Template Setup'); | ||||||||||||||||||
| console.log('=======================\n'); | ||||||||||||||||||
| console.log(`✅ Domain "${domainFromConfig}" is already configured`); | ||||||||||||||||||
| console.log(` Domain folder "${domainFromConfig}" exists and is set up.`); | ||||||||||||||||||
| console.log(' Skipping setup. If you want to re-run setup, remove the domain directory first.\n'); | ||||||||||||||||||
| return; | ||||||||||||||||||
|
Comment on lines
+243
to
252
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): The In the if (!fs.existsSync('touch')) {
return;
}
return;returns in all cases, so the |
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // In CI/non-interactive mode, if touch doesn't exist or not configured, skip silently | ||||||||||||||||||
| if (isNonInteractive()) { | ||||||||||||||||||
| if (!fs.existsSync('touch')) { | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
| // If touch exists but we're in CI, skip silently (don't prompt) | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| console.log('🚀 vNext Template Setup'); | ||||||||||||||||||
| console.log('=======================\n'); | ||||||||||||||||||
|
Comment on lines
241
to
+265
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section has some duplicated code and complex logic. The header is printed in two separate places, and the check for non-interactive mode is split and contains a redundant conditional. This can be refactored to be more readable and maintainable by handling the non-interactive case upfront, and then printing the header once for all interactive scenarios. if (isNonInteractive()) {
// In non-interactive mode (e.g. CI), we should not perform any setup
// or display any output. The script should exit gracefully.
return;
}
// Interactive setup starts here
console.log('🚀 vNext Template Setup');
console.log('=======================\n');
if (domainFromConfig && isDomainFolderConfigured()) {
console.log(`✅ Domain "${domainFromConfig}" is already configured`);
console.log(` Domain folder "${domainFromConfig}" exists and is set up.`);
console.log(' Skipping setup. If you want to re-run setup, remove the domain directory first.\n');
return;
} |
||||||||||||||||||
|
|
||||||||||||||||||
| // Check if already set up | ||||||||||||||||||
| if (isAlreadySetup()) { | ||||||||||||||||||
| console.log('✅ Template is already set up with a domain name'); | ||||||||||||||||||
|
|
||||||||||||||||||
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 use of
|| truewill suppress any exit code fromsetup.js, causing thepostinstallstep to always appear successful even if the setup fails for a legitimate reason (e.g., an invalid domain name provided by a user in an interactive session). This can be misleading as it hides failures. Sincesetup.jsis being modified to exit gracefully in non-interactive environments, this error suppression might not be necessary and could be detrimental in interactive environments. Consider removing|| trueand ensuringsetup.jshas the correct exit behavior for all scenarios.