chinmaye03 / Assignment1

0 stars 0 forks source link

Code review / suggestions #1

Open shrikrishnaholla opened 3 years ago

shrikrishnaholla commented 3 years ago

Good work on the assignment :+1:

I ran the programs, and saw some of the fields appearing as the output. You have mentioned that you would work on getting the others to also be shown. Looking forward to the same :smile:

There was one place I saw a chance for improvement though - could you accept the url as an input rather than creating one program per link? I tried to see whether the logic between pro2, pro3 and pro4 were different, but they weren't. The only difference I could see was with the url being set

$ diff pro2.py pro3.py 
3c3
< Created on Sun May 16 02:35:55 2021
---
> Created on Mon May 17 02:33:10 2021
7a8
> 
13c14
< URL = "https://www.producthunt.com/posts/freshchat-2"
---
> URL = "https://www.producthunt.com/posts/ello-3-0"
53,55d53

This means that any product hunt url can be provided as an input, and the rest of the logic would remain the same

shrikrishnaholla commented 3 years ago

Also, I saw an import of csv module in the file. But I did not see it being used. Could you elaborate on why it was needed?

chinmaye03 commented 3 years ago

Yes sir, trying to figure out to how to display the remaining 2 requirements.As it was within the script tag couldn't figure it out .But will try to get those and let you know .Okay sir will look into taking input as url as well.

Thank you sir.

On Mon, 17 May 2021, 7:21 am Shrikrishna Holla, @.***> wrote:

Good work on the assignment 👍

I ran the programs, and saw some of the fields appearing as the output. You have mentioned that you would work on getting the others to also be shown. Looking forward to the same 😄

There was one place I saw a chance for improvement though - could you accept the url as an input rather than creating one program per link? I tried to see whether the logic between pro2, pro3 and pro4 were different, but they weren't. The only difference I could see was with the url being set

$ diff pro2.py pro3.py 3c3 < Created on Sun May 16 02:35:55 2021

Created on Mon May 17 02:33:10 2021 7a8

13c14 < URL = "https://www.producthunt.com/posts/freshchat-2"

URL = "https://www.producthunt.com/posts/ello-3-0" 53,55d53

This means that any product hunt url can be provided as an input, and the rest of the logic would remain the same

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/chinmaye03/Assignment1/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQFMUNKLXZPRKXUEU7OI34LTOBZCPANCNFSM447PVXDQ .

chinmaye03 commented 3 years ago

Yes sir, actually I tried to implement this program on colab with that example url given , where it was asked to fetch the required details and create a file to write out all the required fields in the file . Later when I used the same import of files to work on the required url in spyder url I forgot to remove that import csv, as in this case the output was required to be displayed in console it was not required. Anyways ,will look into that and commit changes .

On Mon, 17 May 2021, 8:28 am Shrikrishna Holla, @.***> wrote:

Also, I saw an import of csv module in the file. But I did not see it being used. Could you elaborate on why it was needed?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/chinmaye03/Assignment1/issues/1#issuecomment-841945988, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQFMUNPIRXGVIUCRBOXHXD3TOCA4VANCNFSM447PVXDQ .

shrikrishnaholla commented 3 years ago

Looking forward to the changes :)