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

FlakeHell was archived #1817

Open
orsinium opened this issue Jan 12, 2021 · 12 comments
Open

FlakeHell was archived #1817

orsinium opened this issue Jan 12, 2021 · 12 comments
Labels
documentation Docs related task

Comments

@orsinium
Copy link
Collaborator

Sorry, flakehell was archived. it was my the most mentally difficult project, and I don't maintain it anymore. Probably, the WPS documentation should be adjusted accordingly, removing flakehell references. I don't insist, though :)

Luckily, the documentation already has a great section about flake8 --diff which should be enough for most of the cases from the perspective of integrating WPS with a big codebase. Additionally, I can recommend:

  1. reviewdog which supports flake8 out of the box and by default filters violations only for the diff,
  2. GitLab code quality report which shows only violations that are new in the analyzed merge request. There are few flake8 plugins to generate the report: flake8-gl-codeclimate (I used it, works pretty well) and flake8-codeclimate.
@orsinium orsinium added the documentation Docs related task label Jan 12, 2021
@sobolevn
Copy link
Member

@orsinium thanks for working on it! 👍

@orsinium
Copy link
Collaborator Author

No, thank you for your help with the project. You made it possible in the first place :)

@Dreamsorcerer
Copy link
Contributor

Dreamsorcerer commented Feb 4, 2021

We also have #1276 and #1471, which will hopefully make it into 0.16 (flakehell is already removed from the documentation in that first PR).

I've been using that branch for a few months locally now, and it works pretty well.

@sobolevn
Copy link
Member

sobolevn commented Feb 4, 2021

@Dreamsorcerer awesome! Do you have some time to polish it?
I would love to get it merged! 👍

@orsinium
Copy link
Collaborator Author

orsinium commented Feb 4, 2021

BTW, today I was contacted by a team that used flakehell and they decided to maintain a fork:

https://github.com/flakehell/flakehell

So, maybe it will have a second life, we'll see. Anyway, I'll be happy if WPS becomes self-contained in that sense. Or, even better, baseline finally lands into flake8 itself. We already have at least two independent implementations that work, so why not.

@Dreamsorcerer
Copy link
Contributor

@Dreamsorcerer awesome! Do you have some time to polish it?

Possibly, yes. But, will need you to take a look and review first.

@Dreamsorcerer
Copy link
Contributor

Dreamsorcerer commented Aug 19, 2021

I no longer need the baseline functionality, so don't plan to do further development on it. However, the PR is still ready for review, and I'm happy to answer any questions about the code.

I think it is good enough for a public release, but with a few ideas that could be implemented to improve future releases (in particular see #1276 (comment) and the disabled all_change test: https://github.com/wemake-services/wemake-python-styleguide/pull/1471/files#diff-d0914f16c784a2349a4008ccc24c17220012a08741e8c54f290c74dd488efaa1R148).

@sobolevn
Copy link
Member

@Dreamsorcerer thanks a lot for your time and effort! 👍

I am working on another static analysis tool where I consider "baseline" as a core feature.
I am not sure that I will ever add such a complex thing into wps, maybe as a 3rd-party plugin.

@vkurilin
Copy link
Contributor

vkurilin commented May 7, 2022

I would add that flakehell has a community-maintained fork flakehaven. Maybe instead of deleting all mentions, we can replace them with flakehaven and thus support the project?

@Dreamsorcerer
Copy link
Contributor

Dreamsorcerer commented May 7, 2022

I suspect the baseline functionality is still very rudimentary (as in, it will just be too awkward to actually use in a real project with multiple developers). And if they've still not fixed the glaring performance issues, then it would still be totally unusable in my projects (edit: actually, looks like it may have been looked at). I think my closed PR is still much further along with regards to baseline functionality.

@vkurilin
Copy link
Contributor

vkurilin commented May 8, 2022

Not sure if I understand what you're talking about.
At a scale of one hundred thousand lines, flakehell/flakeheaven works quite well, and in terms of performance it is comparable to directly running wemake-python-styleguide through the flake8 command.

What problems there might be with multiple developers, I can't imagine at all. I personally implemented linters through flakehell/flakeheaven on several legacy projects in different companies, and in all cases, the feedback from colleagues was positive.

@Dreamsorcerer
Copy link
Contributor

Not sure if I understand what you're talking about. At a scale of one hundred thousand lines, flakehell/flakeheaven works quite well, and in terms of performance it is comparable to directly running wemake-python-styleguide through the flake8 command.

Not if you install plugins. I think the performance issue may have been atleast partially resolved, but to support the granular controls over enabling/disabling error codes (not something we even wanted to use) it was basically rechecking the code several times or something. So, the performance of flake8 is O(n), while the performance of flakehell is more like O(nm), where n is number of files and m is number of plugins (or something along those lines, it's been awhile). On our project flake8 was taking ~2 minutes to run, while flakehell was taking over 30 minutes (I literally gave up and cancelled it after 30 minutes). That included wemake and several other flake8 plugins on ~30k lines of code.

What problems there might be with multiple developers, I can't imagine at all. I personally implemented linters through flakehell/flakeheaven on several legacy projects in different companies, and in all cases, the feedback from colleagues was positive.

Fair enough. To me, the feature was sufficiently rudimentary that it seemed too cumbersome to use. e.g. Changes frequently result in far too many baseline exceptions reappearing in the output and not being ignored anymore (even from trivial changes).
I was using my branch at my last company, and it was working a lot better. The matching behaviour results in much less previously ignored exceptions reappearing, it updates the baseline after your changes (to avoid multiple small changes reintroducing ignored exceptions), and removes anything from the baseline that is no longer in the code (to avoid accidentally ignoring new exceptions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Docs related task
Projects
None yet
Development

No branches or pull requests

4 participants