-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
CLI: Update CliClient Node dependency to 10+ #2177
Conversation
CliClient/package-lock.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "joplin", | |||
"version": "1.0.149", | |||
"version": "1.0.150", |
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'm not really a Node expert, is this version bump expected when upgrading the node version in package.json
? If not, I can remove this change.
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.
Yep, this (the version bump) is done during the release process. Please revert this change.
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.
You don't have to change the html files because they are generated from the .md files. But it doesn't matter either. Just a thing to remember for the future.
As mentioned earlier inline, please revert the change to the package-lock.json
file. (Never change the file manually, nor commit automatic changes to this file - unless directed by Laurent.)
Other than that... LGTM.
b0cf21e
to
7031ded
Compare
@tessus thanks for the feedback -- I'll remember both points in the future! I've reverted the package-lock.json and the html changes. |
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.
Thank you.
Oops, missed something. I added an inline comment.
fd475be
to
ad2af0d
Compare
@laurent22 This is ready to be merged. The 2 html files don't matter, in fact in this case we don't have to rebuild the web site right away... |
I'd prefer if the commits don't include unnecessary files though. I know it might be confusing that they are in the repo (they are required by GitHub Page) but that's why I've added a big warning on top of the files:
@joeltaylor any chance you could remove the two HTML files please? |
@laurent22 Most certainly, I originally removed them but must not have force pushed. Sorry about that! |
ad2af0d
to
5fb3237
Compare
HTML files removed, apologies for missing that big warning comment. 👍 |
Perfect, thanks @joeltaylor! |
The CliClient does not build correctly with Node 8 anymore and seems to to be the root of installation failures for many reported issues:
In addition, Node 8 is reaching EOL 2019-12-31
Building and installing with Node 10 succeeds and should alleviate the issues.