Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Flake8 from lgtm.yml #7350

Merged
merged 1 commit into from
Jan 28, 2019
Merged

Remove Flake8 from lgtm.yml #7350

merged 1 commit into from
Jan 28, 2019

Conversation

sj
Copy link
Contributor

@sj sj commented Jan 8, 2019

LGTM.com performs its own checks by running queries over source code (all queries are open source on GitHub). It does not show alerts that are generated by Flake.

Unfortunately the provided Flake commands actually return an error, which means that LGTM's Python analysis fails:

[2019-01-05 07:31:29] [build] ++ python3 -m flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics
[2019-01-05 07:31:35] [build] ./regression-tests.recursor-dnssec/test_ECS.py:521:99: F821 undefined name 'cls'
[2019-01-05 07:31:35] [build]             additional = dns.rrset.from_text('ns1.ecs-echo.example.', 15, dns.rdataclass.IN, 'A', cls._PREFIX + '.21')
[2019-01-05 07:31:35] [build]

(https://lgtm.com/projects/g/PowerDNS/pdns/logs/rev/3a0387f7429710a21af7abcc745c71d6932e14be/lang:python/stage:Build%20child%20(3a0387f))

LGTM does not show alerts generated by Flake
@rgacogne
Copy link
Member

rgacogne commented Jan 8, 2019

@sj
Copy link
Contributor Author

sj commented Jan 8, 2019

I think PowerDNS used to have automated code review for pull requests, but it seems that the token for that got revoked:

pdns

(see https://lgtm.com/projects/g/PowerDNS/pdns/ci/)

@Habbie
Copy link
Member

Habbie commented Jan 8, 2019

Yes - LGTM kept commenting as me, and I could not find a way to make it stop doing that. I tried giving it permission via another account but that apparently did not work.

@sj
Copy link
Contributor Author

sj commented Jan 8, 2019

Ah, I see. No worries: the GitHub Apps API integration should be here soon. That comes with built-in support for an LGTM bot account 🙂

@Habbie
Copy link
Member

Habbie commented Jan 8, 2019

Note that we should also fix https://github.com/PowerDNS/pdns/blob/master/regression-tests.recursor-dnssec/test_ECS.py#L521 which is clearly wrong.

This suggests that we should add flake8 testing to something else, like Travis.

@Habbie Habbie merged commit 9817af5 into PowerDNS:master Jan 28, 2019
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.

3 participants