Nostromos / what.ecom

PERN Ecom app for Codecademy's Full Stack path.
MIT License
0 stars 0 forks source link

fix: refactored code and added support of environment variables #61

Closed geekyharsh05 closed 4 months ago

geekyharsh05 commented 5 months ago

This pr fixes #56.

Description:

Code Changes

Reviewer Requests:

Nostromos commented 4 months ago

Not sure how you found this project but a tip - if a project only has one or two people working on it, you should ask before going straight to a PR. I'd also recommend diving deeper and proposing more substantial changes than just changing up imports and dotenv.

Lastly, if you're looking for more open source involvement, you should focus on bigger projects. First, spend 2-3 weeks really learning how a tool works and is built. Then, when people create issues, look for smaller, simple ones like bugs and ask if the maintainers mind if you try to solve. If they say yes, then you try to solve the problem and only put a PR in when you've done that.

geekyharsh05 commented 4 months ago

I'm really sorry for the inconvenience I caused. I understand now that I should have asked for permission before making changes, especially in such a small team setting. I'll make sure to communicate better and ask for guidance in the future. I truly value your feedback and will strive to learn from this mistake. Again, I apologize.

Nostromos commented 4 months ago

No inconvenience and apologies if I came across too harshly! Best of luck in your OSS journey.

BTW, I agree with all the changes you proposed but they're minor and I'm trying to do a few big things before I go back through and clean up.