Skip to content

crawler_valor#85

Open
veniciusgrjr wants to merge 3 commits intoNAMD:masterfrom
veniciusgrjr:master
Open

crawler_valor#85
veniciusgrjr wants to merge 3 commits intoNAMD:masterfrom
veniciusgrjr:master

Conversation

@veniciusgrjr
Copy link

I've created this crawler for Valor. Please check if it's ok. I try to take in count the coments on @lucasmachadorj pull request.

Copy link
Member

Choose a reason for hiding this comment

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

It might be a good idea to break this line to make it more readable. PEP8 suggests 72 with a maximum of 79. I like to follow that whenever possible.

@flavioamieiro
Copy link
Member

I think that, apart from the really small issues I pointed out in line comments, the code looks good and we should merge it.

@flavioamieiro
Copy link
Member

Also, another really important thing: please use 4 spaces instead of tab. I have no real problem with tabs, but mixing spaces and tabs are a bad idea, and our entire code base is using spaces. If you use vim you can use spaces by adding

    set tabstop=4
    set shiftwidth=4
    set expandtab

To your .vimrc.

Copy link
Member

Choose a reason for hiding this comment

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

You don't need to import re nor pandas. It's a good idea to remove these imports.

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