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

Adapt code for flake8 >= 3.8.0 #23

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Adapt code for flake8 >= 3.8.0 #23

merged 1 commit into from
Nov 12, 2020

Conversation

skirpichev
Copy link
Contributor

@skirpichev skirpichev commented Oct 21, 2020

This is a quick fix, which address #22. It seems to be working for me (I tried this on the diofant codebase), but some tests are failing.

I hope, @kataev, you will find a time to review this pull request and help with it's finishing. BTW, Application.__init__() modification may be drasticaly reduced if the upstream removes the hardcoded arguments for manager.OptionManager.

Closes #22

@skirpichev
Copy link
Contributor Author

skirpichev commented Oct 21, 2020

Not sure why CI wasn't started yet. Perhaps, because it's a draft pr?

It seems Travis-CI check is broken in this repo (build was triggered, but build status is not shown in the pr).

@skirpichev skirpichev marked this pull request as ready for review October 21, 2020 08:48
@skirpichev skirpichev marked this pull request as draft October 21, 2020 10:54
@skirpichev
Copy link
Contributor Author

Ok, only py2 tests are broken now, see the build logs.

It seems, this was broken long time ago by newer pytest or hypothesis. Maybe we can require py3? Projects on the Github, using flake8-rst, like pandas, seems to be py3-only.

* Application.__init__() modification may be drasticaly reduced if the
  upstream removes the hardcoded arguments for manager.OptionManager.
* Reimplemented RstFileChecker.reporter()
* Added disable_noqa option with default value for tests

Closes #22
@skirpichev skirpichev marked this pull request as ready for review October 21, 2020 12:44
skirpichev added a commit to skirpichev/diofant that referenced this pull request Oct 24, 2020
@Cabalist
Copy link

I'd love to see this merged. Just burned half an hour troubleshooting this. :)

@skirpichev
Copy link
Contributor Author

@Cabalist, it seems @kataev doesn't care about his pet anymore...

Also, the patch seems to be pretty big. Unfortunately, I doubt we can do anything with this: upstream closing even simple merge requests, which address this issue. Apparently, they won't support some stable API for project's like flake8-rst. They suggest instead insane things like parsing the flake8 output...

@kataev
Copy link
Collaborator

kataev commented Nov 12, 2020

Oh well, let's go merge this :)

@kataev kataev merged commit b878e1d into flake8-docs:master Nov 12, 2020
@kataev
Copy link
Collaborator

kataev commented Nov 12, 2020

I will make release today, sorry for delay!

@skirpichev skirpichev deleted the fix-22 branch November 12, 2020 08:09
@skirpichev
Copy link
Contributor Author

@kataev, maybe you can convince flake8-people to accept something like 458 ? (This one - kills allmost all my modifications in the flake8_rst/application.py) I would expect, that the current using flake8 will be quite fragile, unless we find some way to interact with the upstream. Or, if we reimplement flake8-rst, using public interfaces, which they offer...

@skirpichev
Copy link
Contributor Author

I will make release today, sorry for delay!

@kataev, ping, probably you forgot. Or are there any troubles?

BTW, probably I can help you automate the release process (as I did for the Diofant).

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.

Broken on flake8>=3.8.0
3 participants