-
Notifications
You must be signed in to change notification settings - Fork 2
fix: add package-lock.json for npm ci in docs workflow #92
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Anselmoo <[email protected]>
|
Heads-up: I created #96 to track migrating docs away from legacy dependencies (math renderer and chart wrappers) to avoid the long-term need for 'legacy mode'. Please review #96 and consider the migration plan/checklist; the PR should be re-evaluated to avoid locking the docs into legacy behavior if a maintained alternative is viable. |
Anselmoo
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.
Request changes: prefer migration tracked in #96 over keeping legacy behavior. Please review the candidate migration options and prototype before merging so we avoid long-term legacy mode.
| @@ -0,0 +1 @@ | |||
| legacy-peer-deps=true | |||
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.
Consider avoiding legacy-peer-deps=true if we can migrate dependencies instead; see #96 which tracks migration to KaTeX/MathJax v3 and ECharts + maintained wrappers.
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.
Understood. This PR provides an immediate fix to unblock the failing CI workflow. The proper migration to KaTeX/MathJax v3 and maintained ECharts wrappers should be handled in #96 as a separate effort involving research, prototyping, and testing.
If you prefer to wait for the migration rather than merging this temporary workaround, I can close this PR and the CI fix can be included as part of the #96 migration work.
I've reviewed #96. The migration plan looks comprehensive and is the right approach for long-term maintainability. This PR can be closed if you'd prefer to bundle the CI fix with the dependency migration work in #96, or it can serve as a temporary unblock while #96 is in progress. Let me know your preference.
This PR was intended as an immediate fix to unblock the failing docs deployment workflow. I agree that the migration tracked in #96 is the proper long-term solution. Should I close this PR in favor of addressing the CI failure as part of the #96 migration work instead? |
The docs deployment workflow fails at
npm cistep becausepackage-lock.jsonis missing and explicitly gitignored.Changes
docs/package-lock.json: Generated lockfile (lockfileVersion 3) for deterministic CI installsdocs/.npmrc: Configurelegacy-peer-deps=trueto resolve vitepress 1.5 + markdown-it-mathjax3 5.x peer dependency conflict.gitignore: Removedocs/package-lock.jsonexclusion to allow version control trackingContext
npm cirequires an existing lockfile with lockfileVersion ≥ 1. The.npmrcconfiguration is necessary because vitepress 1.5 declares optional peer dependency on markdown-it-mathjax3 ^4, while package.json specifies ^5.0.0, causing npm ci to fail without the legacy-peer-deps flag.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.