Kill hardcoded arguments for manager.OptionManager - [closed]

[asottile]

In GitLab by @skirpichev on Oct 21, 2020, 01:46

Merges patch-1 -> master

This will help in case if someone subclass Application, as
does flake8-rst right now (see e.g. flake8-docs/flake8-rst#23).

[asottile]

In GitLab by @codecov on Oct 21, 2020, 01:46

Codecov Report

Merging #458 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #458   +/-   ##
=======================================
  Coverage   90.94%   90.94%           
=======================================
  Files          59       59           
  Lines        4242     4242           
  Branches      420      420           
=======================================
  Hits         3858     3858           
  Misses        332      332           
  Partials       52       52           

Impacted Files	Coverage Δ
src/flake8/main/application.py	81.09% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4aeb2e...d8b1c5d. Read the comment docs.

[asottile]

In GitLab by @sigmavirus24 on Oct 21, 2020, 04:33

Application is not a class to be used by plugins or external tools.
Subclassing it will break. Therefore improvements will not be accepted

[asottile]

In GitLab by @skirpichev on Oct 21, 2020, 04:51

Subclassing it will break.

1. I'm not sure what will break this tiny patch. Could you explain?
2. Could you suggest how to reimplement flake8-rst, using the public API? The tool itself seems to be useful and there is a lot of projects, using it (incl. very popular, e.g. pandas).

Therefore improvements will not be accepted

Lets make it clear. If some useful task will be solved only using private API of flake8 - people will use it.

[asottile]

In GitLab by @sigmavirus24 on Oct 21, 2020, 05:36

No, I'm saying there is no guarantee of backwards compatibility. Your
change may not break things but Flake8, the only project that should use
this class doesn't need it.

Flake8-rst may be popular but that doesn't justify using non public classes
that will cause users to experience pain and suffering when that
integration breaks.

Whether people use this or not is no justification for accepting this
change.

[asottile]

In GitLab by @asottile on Oct 21, 2020, 08:25

flake8-rst should not do that, this is not a public api

[asottile]

In GitLab by @skirpichev on Oct 21, 2020, 09:12

flake8-rst should not do that

It would be better, if you can suggest what it should do...

this is not a public api

It's hard to see from the docs, if you ask me. Application class is documented on https://flake8.pycqa.org/

[asottile]

In GitLab by @asottile on Oct 21, 2020, 09:15

flake8 is for python files, flake8-rst simply doesn't make sense

you can access the docs for Application but it is under the "exploring the internals" section of flake8 -- it's explicitly marked as "internal"

[asottile]

In GitLab by @asottile on Oct 21, 2020, 09:16

or use it via subprocess as yesqa does

[asottile]

In GitLab by @asottile on Oct 21, 2020, 09:17

note that #208 tracks exposing a proper api here, but that issue is a bit on the back burner at the moment

[asottile]

In GitLab by @skirpichev on Oct 21, 2020, 09:44

flake8 is for python files, flake8-rst simply doesn't make sense

Well, it certainly has sense for me or for pandas devs. Why the idea of having doctests formatted as other project's code doesn't make sense?

or use it via subprocess as yesqa does

Thank you, I'll think about.

IIUIC, you suggest filter out doctests code blocks to the temp file, then feed it to the flake8? Maybe this will work, but I don't sure that it's possible in this way - to fix line/column numeration at the end.

[asottile]

In GitLab by @asottile on Oct 21, 2020, 10:06

should be trivial to += lineno and += col_offset -- not sure what you mean

[asottile]

In GitLab by @ericvw on Oct 21, 2020, 10:16

For linkage, #641 was closed due to flake8-rst using the non-public flake8 APIs.

[asottile]

In GitLab by @skirpichev on Oct 21, 2020, 10:31

should be trivial to += lineno and += col_offset

I doubt. Parsing the flake8 output (raw strings) - seems to me even more ugly solution (think about --show-source case, e.g.), then using the private API.

[asottile]

In GitLab by @asottile on Oct 21, 2020, 10:56

consider flake8-json then 🤷

[asottile]

In GitLab by @skirpichev on Oct 21, 2020, 11:30

consider flake8-json

Well, it doesn't produce a valid json with --show-source. Not sure if this is a bug or feature.

[asottile]

In GitLab by @asottile on Oct 21, 2020, 11:36

locked this merge request