Copilot commented on code in PR #1968:
URL: https://github.com/apache/apisix-website/pull/1968#discussion_r2468539684
##########
.github/workflows/deploy.yml:
##########
@@ -108,6 +108,32 @@ jobs:
run: |
yarn build
+ - name: Show build logs on failure
+ if: failure()
+ run: |
+ LOG_DIR="/tmp/apisix-website-build-logs"
Review Comment:
The log directory path is hardcoded here and in the script. If this path
needs to change, it must be updated in multiple locations. Consider using an
environment variable or a shared configuration to maintain consistency.
##########
scripts/generate-website.js:
##########
@@ -1,8 +1,43 @@
const Listr = require('listr');
const util = require('util');
+const fs = require('fs');
+const path = require('path');
const { chdir } = require('node:process');
const exec = util.promisify(require('node:child_process').exec);
+// Detect CI environment
+const isCI = process.env.CI === 'true' || process.env.GITHUB_ACTIONS ===
'true';
+
+// Control whether to show logs locally (default: true for convenience)
+// Set SHOW_BUILD_LOGS=false to disable local log output
+const showLogsLocally = process.env.SHOW_BUILD_LOGS !== 'false';
+
+// Log directory for build outputs
+const logDir = '/tmp/apisix-website-build-logs';
Review Comment:
The hardcoded path `/tmp/apisix-website-build-logs` is duplicated in both
the script and the GitHub workflow file. Consider defining this as a shared
constant or environment variable to avoid inconsistencies if the path needs to
change in the future.
```suggestion
// Log directory for build outputs (can be overridden by environment
variable)
const logDir = process.env.APISIX_WEBSITE_BUILD_LOG_DIR ||
'/tmp/apisix-website-build-logs';
```
##########
scripts/generate-website.js:
##########
@@ -17,34 +52,118 @@ const tasks = new Listr([
},
{
title: `Build website's all parts`,
- task: () => Promise.allSettled([
- exec('yarn run build:blog:zh', { stdio: 'ignore' }),
- exec('yarn run build:blog:en', { stdio: 'ignore' }),
- exec('yarn run build:doc', { stdio: 'ignore' }),
- exec('yarn run build:website', { stdio: 'ignore' }),
- ]),
+ task: async (ctx) => {
+ const buildSteps = [
+ { name: 'blog-zh', cmd: 'yarn run build:blog:zh' },
+ { name: 'blog-en', cmd: 'yarn run build:blog:en' },
+ { name: 'doc', cmd: 'yarn run build:doc' },
+ { name: 'website', cmd: 'yarn run build:website' },
+ ];
+
+ // Execute all builds in parallel, capturing output
+ const results = await Promise.allSettled(
+ buildSteps.map((step) => exec(step.cmd)),
+ );
+
+ // Check for failures and save logs to files
+ const failures = [];
+ const logFiles = [];
+
+ results.forEach((result, index) => {
+ const step = buildSteps[index];
+ const logFilePath = path.join(logDir, `${step.name}.log`);
+ let logContent = '';
+
+ if (result.status === 'fulfilled') {
+ // Write raw stdout and stderr without extra formatting
+ if (result.value.stdout) {
+ logContent += result.value.stdout;
+ }
+ if (result.value.stderr) {
+ if (logContent) logContent += '\n';
+ logContent += result.value.stderr;
+ }
+ } else {
+ failures.push(step.name);
+ // Write raw stdout and stderr for failed builds
+ if (result.reason.stdout) {
+ logContent += result.reason.stdout;
+ }
+ if (result.reason.stderr) {
+ if (logContent) logContent += '\n';
+ logContent += result.reason.stderr;
+ }
+ // Add error message at the end if available
+ if (result.reason.message &&
!result.reason.stderr?.includes(result.reason.message)) {
Review Comment:
The check `!result.reason.stderr?.includes(result.reason.message)` may fail
if `result.reason.stderr` is undefined (returns true) or if the error message
appears as a substring within a larger message. This could lead to duplicate
error messages in the log file. Consider checking if `result.reason.stderr`
exists before using `.includes()`, or simplify to always append the error
message with a clear separator.
```suggestion
if (result.reason.message) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
