-
Notifications
You must be signed in to change notification settings - Fork 5k
chore: generate cli help in-build #38945
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
5688d52 to
6670ca2
Compare
6670ca2 to
e4a5a2f
Compare
manimovassagh
left a comment
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.
Great job
|
|
||
| const fileName = path.resolve(__dirname, '../packages/playwright/lib/mcp/terminal/help.json'); | ||
| console.log('Writing ', path.relative(process.cwd(), fileName)); | ||
| fs.writeFileSync(fileName, JSON.stringify(generateHelpJSON(), null, 2)); |
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.
Just a recommendation: Consider adding error handling here to make build failures more obvious:
try {
fs.writeFileSync(fileName, JSON.stringify(generateHelpJSON(), null, 2));
console.log('✓ Successfully wrote', path.relative(process.cwd(), fileName));
} catch (error) {
console.error('✗ Failed to write help file:', error.message);
process.exit(1);
}This will make it clearer if the file write fails during the build process.
| nohup.out | ||
| .trace | ||
| .tmp | ||
| .playwright-cli |
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 we also add the generated help file here?
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.
Since help.json is now auto-generated during build, it probably shouldn't be checked into version control.
Test results for "tests 1"8 failed 7 flaky34658 passed, 695 skipped Merge workflow run. |
Test results for "MCP"9 failed 3105 passed, 305 skipped Merge workflow run. |
No description provided.