edgi-govdata-archiving / edgi-hubot-halpy

Heroku-backed chatbot-friend who lives in EDGI Slack
https://edgi-hubot.herokuapp.com/
3 stars 0 forks source link

Dependencies update #44

Closed Frijol closed 5 years ago

Frijol commented 5 years ago

This PR has two commits that update dependencies:

  1. A simple npm install to update package-lock
  2. npm suggested further dependency updates via npm audit fix (note that there are more updates the npm audit suggest but they require manual review)

When I run npm start and halpy help it all seems fine– is there more testing that needs to be done before a merge?

Mr0grog commented 5 years ago

Also thank you for this! The notification had been bugging me, and I’d been meaning to try and fix it myself, but never found the time.

Mr0grog commented 5 years ago

Oh, hmmmm, I just realized I was running this with the wrong version of Node.js (it’s supposed to be 4.x and I was using 10.15.3). I actually can’t even install the current master branch dependencies on my local machine (iconv fails to build, and I’m betting the version in package.lock is not compatible with Node 4 on MacOS Mojave). Did you do this with Node v4?

If not, maybe it makes sense to just update to v10 in package.json. Everything seems like it should run fine in an actually current and supported version of Node.js.

Frijol commented 5 years ago

Oh hm. I always forget to check which version of Node I'm running. Looks like v11.8.0 which is a weird choice.

Frijol commented 5 years ago

Ok, I switched to 10.15.3 (current LTS), re-checked the npm install (no change to package-lock), noted the change in package.json. Good catch, I think this is the right way to go. Good to merge as far as you know @Mr0grog?

Frijol commented 5 years ago

:/ pulled it down just to check and now it's not running locally for me (on 10.x or 11.x). Is that true for you too @Mr0grog ? says it's erroring on a Heroku config but it doesn't make sense to me that it would have worked before if that's the issue

Mr0grog commented 5 years ago

Hmmm, it’s running fine for me…

> git checkout master; git pull
# Remove node_modules because of Node.js version change
> rm -rf node_modules
> npm install
> npm start

Deploy logs in Heroku show success, and it responds to me in Slack.