Skip to content

Conversation

@NT-me
Copy link

@NT-me NT-me commented Jun 3, 2019

What type of change does this PR introduce?

  • Bugfix
  • Feature
  • Refactor
  • Documentation
  • Not Sure?

Does this PR introduce breaking changes?

  • Yes
  • [X ] No

Description:

With this package you can ask Leon about the weather in all the cities of the world.
It's not full finish because it's miss test and little things.

@louistiti
Copy link
Member

Hello @gnouf1,

Thanks for that PR! I'll take a look at that later.

In the meantime, let me know when you have finished it and do not hesitate to ask any questions.

@NT-me
Copy link
Author

NT-me commented Jun 4, 2019

Okay, i'm gonna add config file and future weather.
And I just want to say : I take a great pleasure when I work on features for leon :p

@louistiti
Copy link
Member

I'm happy to see that you enjoy working on new Leon features!

If you have any feedback or see things to improve, do not hesitate to reach me out, I'd love to hear them.

@NT-me
Copy link
Author

NT-me commented Jun 5, 2019

Okay no problem !
After this features i gonna make other i guess, i have time this month.

For me apart from the JS test it seems good for a version 1.0.0.

@facorazza
Copy link
Contributor

I was writing about the same package. I'd like to point out a few suggestions:

  • To me the name of the package sounds a bit redundant, perhaps we could call it simply weather;
  • English comments would be appreciated;
  • Perhaps we could use the Python wrapper for OpenWeatherMap: PyOWM;
  • Support for Celsius and Fahrenheit scales;
  • Set the api token in the settings.json file along with other settings like deciding which temperature scale to use etc.

@NT-me
Copy link
Author

NT-me commented Jun 5, 2019

English comment are just a little forget from me, sorry :)
And for settings.json I totaly agree with you !

@NT-me
Copy link
Author

NT-me commented Jun 6, 2019

English commentary are up, config file is also OK, Farenheit and Kelvin are now usuable !

@louistiti
Copy link
Member

Awesome! 👏

It would also be better if your commit messages are in English to ensure that most people understand ☺️

@NT-me
Copy link
Author

NT-me commented Jun 6, 2019

Of course !
Sorry i'm new on github (and python) and I learn how to use it :)
From now on I'll be careful about that :)

@NT-me
Copy link
Author

NT-me commented Jun 6, 2019

Test up :)

@NT-me
Copy link
Author

NT-me commented Jun 7, 2019

What are finale steps ? :)

@louistiti
Copy link
Member

I'll review it later, thanks! 😄

In the meantime, feel free to pick up another module you would like to develop or whatever you think is good and open an issue to discuss about it first.

@NT-me
Copy link
Author

NT-me commented Jun 8, 2019

Little update, now he use multilanguage weather :)

@NT-me
Copy link
Author

NT-me commented Jun 10, 2019

Merge with other weather package

@NT-me NT-me closed this Jun 10, 2019
@louistiti
Copy link
Member

#108.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants