feat(openweather): implementation to API openweather #762
Conversation
jmellano
force-pushed
the
feat-openweather
branch
3 times, most recently
from
774e6b9 to
828e29f
Compare
Pierre-Gilles
changed the title
|
Nice! I put [WIP] in the title to indicate that this PR is still work in progress. Feel free to mark this PR as "Ready for review" when the PR is finished :) Thanks for your work 🙌 |
jmellano
force-pushed
the
feat-openweather
branch
2 times, most recently
from
314e2ad to
2c84fcf
Compare
Codecov Report
@@ Coverage Diff @@
## master #762 +/- ##
==========================================
+ Coverage 92.38% 92.71% +0.32%
==========================================
Files 419 419
Lines 5427 5421 -6
==========================================
+ Hits 5014 5026 +12
+ Misses 413 395 -18
Continue to review full report at Codecov.
|
jmellano
force-pushed
the
feat-openweather
branch
3 times, most recently
from
b2c44b8 to
12a844c
Compare
jmellano
changed the title
|
Quand tu dis "Feel free to mark this PR as "Ready for review" when the PR is finished"... ca consiste a retirer le WIP du titre, on est bien d'accord ? Pour le fail sur le codecov/patch... je pense qu'il faut en discuter mais en supprimant la partie "dark sky" ca doit revenir au niveau voulu |
|
Tu retires WIP du titre + tu demandes une review =>
Sinon je ne suis pas notifié et je n'ai aucune idée qu'il faut que je vienne ici :)
Tu peux retirer la partie darksky :) Il faut que cette PR ait le bon code coverage sinon elle ne peut pas être mergée ! |
jmellano
force-pushed
the
feat-openweather
branch
4 times, most recently
from
b346e91 to
6c83339
Compare
|
@Pierre-Gilles pour la partie "+ tu demandes une review "... je crois que je n'ai pas les droits... j'ai rien au niveau "Reviewers" qui m'indique que je peux sélectionner qqn... |
|
Ah mince! Bon je me suis auto ajouté alors :) |
There was a problem hiding this comment.
Thanks for this great PR ! 👏
This is really good work ! I added a few feedbacks, if you can add a look?
Note: there is a conflict with some recent changes on the way we translate services, see #805
| @@ -169,7 +169,7 @@ | |||
| "humidity": 0.99, | |||
| "pressure": 1005.09, | |||
| "datetime": "2019-05-09T04:27:57.000Z", | |||
| "units": "si", | |||
There was a problem hiding this comment.
Be careful, there are still mention of "si" in the code (search in the code to find where)
There was a problem hiding this comment.
? weird...in my branch there is no more mentions on "si"...even in demo.json file it's "metric"
jmellano
force-pushed
the
feat-openweather
branch
from
a32c687 to
60cc449
Compare
TODO : Have a decision about DarkSky (keep it and enable weather with 2 services) or remove it
"si", 'si' -> 'metric'
jmellano
force-pushed
the
feat-openweather
branch
from
08f1d15 to
887c547
Compare
Salut,
j'ai vu que vous vouliez changer la partie météo de darksky vers openweather.
Du coup pour me remettre dans Gladys et surtout la v4, j'ai essayé d'implémenter qqch.
Ce n'est pas fini (cf. todo list en bas), mais je voudrais juste un avis sur le fait que cette feature est toujours identifiée comme prioritaire (pour la sortie d'une RC) et que le code va dans la bonne direction pour toi. (#726 )
Pull Request check-list
To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:
npm teston both front/server)npm run eslinton both front/server)npm run prettieron both front/server)front/src/config/demo.json) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.
Description of change
Please provide a description of the change here. It's always best with screenshots, so don't hesitate to add some!