Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: support monocart report #521
feat: support monocart report #521
Changes from 6 commits
2308d4a
caae21f
1a7d020
59a6161
4e856a2
28be91c
6c20c54
7e60c5c
aecd2b3
2192d25
1a82a95
4dbfe35
27966b3
5aa4422
9566904
2218b53
237d262
4830f62
1d2ff88
85303b4
714cdd6
c88abbf
6fd00e5
4b0947a
67bbd57
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
we should move this into a try/catch.
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.
Perhaps we should move if into a try/catch in a separate monocart.js helper file, then we can mock that file to test the help output.
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.
If we were unable to require Monocart, we should print the install instructions here and exit with 1.
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.
I have made the following improvements based on your suggestion
getMonocart ()
import("xxx")
function to load the lib asynchronously. It should be good enough without creating a new help file, and added new test case to cover it (rename the lib entry file can mock it).Please review, thanks.
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.
👍 I prefer now that we're sharing some of the same behaviour in
report.js
.