Bug in Weather Units for Broadcasted Notification

[skpanagiotis]

This PR resolve Issue number #3419 .

I have added the method convertWeatherObjectToImperial() which converts the units of the notificationPayload to imperial if needed, in order to pass the object in sendNotification().

[khassel]

Thanks! Can you please add an entry to the CHANGELOG.md file? (otherwise the tests will fail ...)

[khassel]

if you need help to fix the lint errors in the failed test let me know

[sdetweil]

Some of the problem is doing npm run install-mm doesn’t install the git commit hooks

[skpanagiotis]

I resolve eslint errors.
Is there a possibility that the failed tests are due to my code?

[khassel]

from the test: "Error: Uncaught [SyntaxError: Identifier 'WeatherObject' has already been declared]\n"

yes, you introduced const WeatherObject = require("./weatherobject"); but in the tests we have in file weather_object_spec.js the same variable name const WeatherObject = require ... so renaming one of them should solve this

[khassel]

looks like a typo:

convertPrecipitationToInch vs. this.convertPerticipationToInch

[skpanagiotis]

Thanks very much for your time and help, my mistake is that I skipped running tests, because I was working on Windows system.

[rejas]

this else code does the same as the if part?
and why not use the convertPrecipitationUnit method?

[skpanagiotis]

the code inside if converts cm to inch and the code inside else converts mm to inch.
The reason that I added convertPrecipitationToInch () method is that convertPrecipitationUnit() returns a string, and I think that maybe we need the raw value of precipitation.

[rejas]

sorry for late reply and my first wrong response...

i get your point, will add a small improvement myself and approve

[rejas]

sorry for late reply and my first wrong response...

i get your point, will add a small improvement myself and approve