-
Notifications
You must be signed in to change notification settings - Fork 313
Development
: Remove yargs parser
#10963
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
WalkthroughThe "yargs-parser" version "22.0.0" entry was removed from the "overrides" section of the package.json file. No other dependencies or overrides were modified, and there were no changes to exported or public entities. Changes
Suggested labels
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
1-207
: Regenerate lockfile and update documentation
After removing an override, it’s best practice to regenerate your lockfile (npm install
oryarn install
) and clear any CI cache so pipelines pick up the change. Also consider adding a brief note to yourCHANGELOG.md
or project docs explaining whyyargs-parser
was removed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: server-tests
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: Analyse
🔇 Additional comments (1)
package.json (1)
87-116
: Approve removal ofyargs-parser
override
Theyargs-parser
entry has been removed from theoverrides
section to resolve the ESM-only compatibility issue. Confirm that no parts of your codebase still referenceyargs-parser
and that the build now completes successfully without this override.Run the following to catch any lingering references:
rg -n '"yargs-parser"'
End-to-End (E2E) Test Results Summary
|
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.
E2E tests are pasing again and client starts locally again
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.
thanks 👍
build + e2e + client test are passing
I don't see any usages of it anyway
In case you encounter issues locally, make sure to update your node version. |
Checklist
General
Motivation and Context
The client build fails locally with
Description
Removed the yargs parser. It's esm-only since v22.0 which isn't compatible with Artemis.
If it's used indeed, we have to downgrade.
Steps for Testing
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Summary by CodeRabbit