Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

Add flake8 without RecursionError #223

Closed

Conversation

cclauss
Copy link

@cclauss cclauss commented Jul 19, 2018

Description

Another approach to @leezu #219 that is based on some of the ideas in the review of #213 that were lost when that PR was simplified. Our goal is to add flake8 tests while avoiding a RecursionError. We do this by creating three different steps in Makefile that can be run separately or together via a unified make lint command. This allows contributors to run these tests separately or together.

Checklist

Essentials

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have are test coverage
  • Code is well-documented

Changes

  • Add make flake8, make restruc, and make lint to Makefile

Comments

@cclauss cclauss requested a review from szha as a code owner July 19, 2018 04:45
@cclauss cclauss force-pushed the add-flake8-without-RecursionError branch from 320e4ef to 3f7919f Compare July 19, 2018 04:50
@szha szha requested a review from leezu July 19, 2018 04:56
@cclauss cclauss force-pushed the add-flake8-without-RecursionError branch from 3f7919f to 8bd25ec Compare July 19, 2018 04:58
@leezu
Copy link
Contributor

leezu commented Jul 19, 2018

Thanks @cclauss . I have pushed your commit in #219 , as the CI doesn't allow you updating the Jenkinsfile for security reasons. But the RecursionError remains. I believe it is an issue with the conda setup, though I am not yet sure what exactly.
Sorry for not answering to your cleaned up proposal in #213 yet (ie. what you have put in to a commit here now). For #219 it was not intended to merge yet but to debug why the CI fails. I mean, your contribution in #213 was very welcome and valued, just as I'm still debugging the conda setup, not yet included.

@cclauss cclauss force-pushed the add-flake8-without-RecursionError branch from 8bd25ec to 0a965f5 Compare July 19, 2018 05:04
@cclauss
Copy link
Author

cclauss commented Jul 19, 2018

OK.... Closing.

@cclauss cclauss closed this Jul 19, 2018
@cclauss cclauss deleted the add-flake8-without-RecursionError branch July 19, 2018 05:09
@leezu
Copy link
Contributor

leezu commented Jul 19, 2018

@cclauss Thanks for your contribution. I merged your commit together with a fix for the RecursionError. The issue was that flake8 tried to check all files in the local conda environment, which is in ./conda in the checked out directory.

@leezu
Copy link
Contributor

leezu commented Jul 19, 2018

Also see PyCQA/pyflakes#347

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants