-
Notifications
You must be signed in to change notification settings - Fork 1
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
Update the minimum supported nodejs version to v10 and fix bugs #28
Conversation
Note: May contain some bugs, mainly due to two reasons 1. replacing replaceAll with replace 2. replace ? for || There are also some non-dangerous replacements For example, appending the fs-extra module to Promise fs-related operations
May need a better way to replace replaceAll, as for ??, may need a utility function? |
First of all: Thank you very much for your contribution. I'll review this asap. I believe that the replacement for With |
Alright. Results from my initial review (I'll "GitHub-Review" when I've had the chance to test this more thoroughly, this is just the state of my review, as of now 🙂):
|
…rred, but the upstream cannot be updated because of this conflict)
Code Climate has analyzed commit e9dd5ba and detected 0 issues on this pull request. View more on Code Climate. |
It has been modified. In addition, I merged the upstream modification (ie the latest commit of this repository) and resolved the conflict. |
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.
LGTM. Thank you for your contribution! 😄
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.
LGTM, I'll go ahead and merge it 🙂
Thank you very much for your contribution.
Co-authored-by: rxliuli <rxliuli@gmail.com>
Co-authored-by: rxliuli <rxliuli@gmail.com> Closes: #20
Note: May contain some bugs, mainly due to two reasons
There are also some non-dangerous replacements
For example, appending the fs-extra module to Promise fs-related operations
I created this PR to illustrate how easy it is to support nodejs10. I simply modified a few files. may cause some errors, but I have used this version to generate personal project documents, and it seems that there is no problem at the moment: https://util.liuli.moe/
ref: #20