-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Improving the code and fixing some bugs #1
Conversation
Thanks! Can you explain how the race would have worked? I didn't see any errors with the |
// Use waitgroup so we can keep track of tasks | ||
columns := []string{*name, *address, *phone, *email} | ||
headers := []string{"name", "address", "phone", "email"} | ||
// url and id are added as the first two rows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/rows/columns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(fixed on master)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call it columns, because those are the column names ... but it's really up to you
I really like the way the Scrape function was rewritten. |
There was no data race, sorry. But you were leaking go routines, which didn't have any effect on your binary since it's short lived |
So would you mind if I made a video of me reviewing this code and posted it on YouTube? 🙏 |
Yes @campoy - go for it! I sent you an email about it too. |
You had data races 😄
The order of columns and headers did not match
Too many files for what it is, I think now is more readable
I'll use this code in a video where I show how I improved it, if you don't mind!