Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leon weather #103

Closed
wants to merge 17 commits into from
Closed

Leon weather #103

wants to merge 17 commits into from

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