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

Exercise 1 completed #24

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

litmus-zhang
Copy link

Exercise 1 completed with single threading and multithreading

Copy link

@vinaysinghDXC vinaysinghDXC left a comment

Choose a reason for hiding this comment

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

files downloaded for engineering need to seprate from zip files the csv files

Copy link

@vinaysinghDXC vinaysinghDXC left a comment

Choose a reason for hiding this comment

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

Feedback on Code Quality

Great work on this pull request! The code looks clean and well-organized. I appreciate the effort you've put into this.

Suggestions

  1. Error Handling: Ensure that error handling is robust. Are there any edge cases or unexpected scenarios that need to be handled? Think about potential exceptions and how they should be caught or propagated.

Manual Adjustments

I noticed that the configuration file paths are hard-coded. To make the project more flexible, consider allowing users to specify custom paths via environment variables or command-line arguments. This way, users won't need to modify the code directly.
Post downloading the web scrapped files the zip files have to be mannually unzipped and unloaded. There is a chance of improvement there

Keep up the good work, and feel free to address these suggestions at your convenience! 🚀

@vinaysinghDXC
Copy link

Approved

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.

2 participants