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

Adds weather Alert notification system #16

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

Telomelonia
Copy link
Contributor

@Telomelonia Telomelonia commented Oct 8, 2024

solves #15
Adds the weather alert notification py script

I used https://api.openweathermap.org/data/2.5/find

@Telomelonia
Copy link
Contributor Author

@king04aman plz review this

@king04aman
Copy link
Owner

Thank you for your contribution! I really appreciate your work on the weather alert notification system—as it's a great start. I have a few suggestions that could help enhance it even further:

  1. Security - Please use environment variables to store the API key instead of hard-coding it.
  2. Error Handling - Enhance error handling by catching specific exceptions to better identify issues.
  3. Input Validation - Add validation for user inputs to ensure they are numeric and sensible.
  4. Function Documentation - Include docstrings for your functions to improve clarity.
  5. Logging - Consider using a logging framework instead of print statements for better output management.

These tweaks would make the code more secure and easier to maintain. I’m excited to see your updates!

@Telomelonia
Copy link
Contributor Author

@king04aman
made the required changes

Copy link
Owner

@king04aman king04aman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a README file significantly increase project clarity and understanding.

@Telomelonia
Copy link
Contributor Author

Telomelonia commented Oct 17, 2024

Adding a README file significantly increase project clarity and understanding.

Done 👍

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

Successfully merging this pull request may close these issues.

2 participants