gocolly / colly

Elegant Scraper and Crawler Framework for Golang
https://go-colly.org/
Apache License 2.0
23.2k stars 1.76k forks source link

Warnings from golint #81

Closed peterhellberg closed 6 years ago

peterhellberg commented 6 years ago

ID is part of the list of common initialisms used by the golint tool and thus we get warnings like this:

Unfortunately fixing this problem would change the exported types Collector and Request. Would a change like this even be considered?

asciimoo commented 6 years ago

I wasn't aware of this golint rule when I added Id attributes.

Would a change like this even be considered?

Personally I don't think this would be a major issue (goreportcard.com doesn't even test this rule: https://goreportcard.com/report/github.com/gocolly/colly#golint), so keeping it as Id is fine to me, but if you find it better to change, we can do it. As I see no public code uses these attributes: https://github.com/search?p=1&q=gocolly+id&type=Code&utf8=✓.

What do you think?

peterhellberg commented 6 years ago

I'd would absolutely prefer if the code in this project didn't give any warnings when running tools like golint, go vet, etc. :)

asciimoo commented 6 years ago

Ok, I'll do it especially because your previous PR introduced an ID. =) I'm also adding the golint check to travis.

peterhellberg commented 6 years ago

I was debating myself if I should have added more warnings by adding an Id function or not… decided against it.

peterhellberg commented 6 years ago

You might want to remove the os-section and add sudo: false in the .travis.yml

Then Travis-CI will use the Docker builders instead of VM builders (should be much faster)

https://docs.travis-ci.com/user/reference/trusty/#Container-based-with-sudo%3A-false