Remove flake8 from travis

[fujiisoup]

• Closes Code review bots? #1912

The removal of flake8 from travis would increase the clearer separation between style-issue and test failure.

[shoyer]

If we're confident that stickler is serving

[fmaussion]

Yes, I would also wait a little more to see if we are happy with stickler

[fujiisoup]

OK. Let's wait a little more.
It looks the rules are a little bit difference between flake8 in travis and stickler-ci.

[Zac-HD]

I would be very disappointed to remove the flake8 check from Travis - in my experience, style guides are only applied consistently if they are enforced by CI.

Ensuring that each pull maintains our standards is much less burdensome than regular "big push" efforts to get back to the standard (see eg #1741 and #1824 - it took two people!), and avoids periods of lower code quality.

In short, I think that Travis is the right place to put these checks - review bots are a complement, not a substitute.

[fujiisoup]

@Zac-HD, I don't mean to drop CI-based flake8 checking.
We are trying to move stickler-ci, which is a CI specific for flake8 check.

I thought if stickler works fine, travis could concentrate on testing,
but stickler does not behave as what I have expected.
I agree that it is not a good idea to remove flake8 check from travis at this moment.

I think that in the early stage of the development it would be nicer if we could easily distinguish between testing failure and style inconsistency, but travis just says succeeded or not.
(it is possible to find it if we look deep inside the travis's log, but it requires some burden).

Do you have any idea to realize more explicit failure reporting?

[Zac-HD]

home-assistant uses https://houndci.com, and it seems to work pretty well for them - it just leaves a review with any flake8 comments at the relevant lines of the pull.

Note that while HoundCI is used to provide faster and more readable feedback to contributors, they still check flake8 in Travis to ensure everything is fixed before merging.

[fujiisoup]

Thanks for the information.
I do not have much experience about HoundCI but if it works well, it could be an alternative of stickler.

they still check flake8 in Travis to ensure everything is fixed before merging.

But why is travis so special?
HoundCI reports any flake8 violation not only as comments but also in All checks have passed part of PR, as travis does.
I think that maintainers can check any flake8 violation just from HoundCI, before merging.

[max-sixty]

I share @fujiisoup 's perspective

But if people as wise as @Zac-HD disagree, let's leave Travis running for 1-2 months, see if it's helpful, and if it's not then we can turn it off. It's not immediately costly, and we'll gain understanding over the coming months.

[Zac-HD]

Last time I used it Hound didn't update the PR status, so if so that removes one of my objections 😄 As a maintainer I prefer automating checks - I make mistakes, especially when tired, and it's nice knowing that the system has my back.

Hound only runs checks in the diff range though, so I think it's still valuable to keep running flake8 ourselves. Trying both for a few months before making a final decision sounds like a good idea to me!

[max-sixty]

FWIW I think stickler is working v well, +1 to merge this

[jhamman]

@fujiisoup - I think you can merge this whenever you're ready.