jclarke0000 / MMM-DarkSkyForecast

Magic Mirror module to display weather information using the Dark Sky API
84 stars 64 forks source link

Typo in the request call in node_helper.js #5

Closed michael5r closed 5 years ago

michael5r commented 5 years ago

I checked to see if you'd fixed this in the 1.6 tag, but it's still in there.

In your node_helper.js, you're doing this:

request({url: url, methid: "GET"}, function( error, response, body) {

method is misspelled.

You also have request as a dependency in your package.json - it's not necessary as it's already a dependency in MagicMirror's package.json file.

To be honest, you don't need any of the dependencies in your package.json file at all - moment and request are both production dependencies of MagicMirror and npm shouldn't be included at all. Might be nice to remove them, so people don't have to do npm install for this module.

jclarke0000 commented 5 years ago

I fixed the typo in the request call, but I'm going to leave request and moment.js as dependencies. If I remove them as dependencies, then my module will cease to work if MagicMirror removes either of these as dependencies.

michael5r commented 5 years ago

If MagicMirror for some reason decided to remove request and moment as production dependencies, they'd have to publish a new major version and label it as a breaking change.

(They'd also have to rewrite a ton of their core code which relies on both of these dependencies which means that the API would have changed & your module would most likely not work anyway.)

In regards to the other issue you closed, npm is the default package manager for Node, is automatically installed when you download Node and has nothing to do with your module.

You can't specify a specific version of npm that people should be using because the version they're using depends on the version of Node they have installed.