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

Add flake8 linter workflow #923

Merged

Conversation

drikusroor
Copy link
Contributor

Background

I've recently opened a PR (#865) that will add automated unit tests using Github workflow for every PR and commit on the master branch. The suggestion was made to also add flake8 linting, though in a separate PR, which you are looking at right now. I think it'd be a good idea to merge that other PR first and then append the workflow by merging this one.

Changes

  • Add flake8 linting
  • Add a flake8 linting job to the workflow (continue after error is set to true as there were still quite some linting issues)
  • Rename the unit_tests.yml to ci.yml as it now contains more jobs than just unit tests

Documentation

I've added a small section to README.md that explains how to run the flake8 linter.

Test Plan

If the Python CI workflow succeeds the changes from this PR work as expected.

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts. (prompts N/A)
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively. (needn't be comprehensive)
  • I have not snuck in any "extra" small tweaks changes

@drikusroor drikusroor mentioned this pull request Apr 12, 2023
5 tasks
@richbeales richbeales added the enhancement New feature or request label Apr 12, 2023
Copy link
Contributor

@nponeccop nponeccop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also suggest adding flake8 and the tests in a different PRs, and using the following small list of pep8 fixes:

python -m flake8 --select E303,W293,W291,W292,E305

This way we will start having pep8 but we won't break many PRs, and we gradually extend the list introducing minimal breakage.

.gitignore Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
tests/context.py Outdated Show resolved Hide resolved
nponeccop
nponeccop previously approved these changes Apr 12, 2023
@drikusroor
Copy link
Contributor Author

I also suggest adding flake8 and the tests in a different PRs, and using the following small list of pep8 fixes:

python -m flake8 --select E303,W293,W291,W292,E305

This way we will start having pep8 but we won't break many PRs, and we gradually extend the list introducing minimal breakage.

The unit tests itself are added in a separate PR (here #865) already. I don't know if that's what you meant.

As for the python -m flake8 --select E303,W293,W291,W292,E305 command: Are you suggesting to first use this command and then in later PRs we gradually be more strict about it, or am I understanding you wrong? :-)

@nponeccop
Copy link
Contributor

I don't know if that's what you meant.

Well, I apologize then. Yes as for getting stricter you are right.

@drikusroor
Copy link
Contributor Author

I don't know if that's what you meant.

Well, I apologize then. Yes as for getting stricter you are right.

No worries, I was just wondering 😅

@richbeales
Copy link
Contributor

Hi, your unit test PR has been merged, please can you resolve the conflicts in this one? Thanks

@richbeales richbeales merged commit 079daf7 into Significant-Gravitas:master Apr 12, 2023
@drikusroor drikusroor mentioned this pull request Apr 12, 2023
5 tasks
SquareandCompass pushed a commit to SquareandCompass/Auto-GPT that referenced this pull request Oct 21, 2023
…t-Gravitas#923)

* merging

* clean commit

* Delete mylearner.py

This file is not needed.

* fix py4j import error

* more tolerant cancelling time

* fix problems following suggestions

* Update flaml/tune/spark/utils.py

Co-authored-by: Li Jiang <bnujli@gmail.com>

* remove redundant model

* Update test/spark/custom_mylearner.py

Co-authored-by: Chi Wang <wang.chi@microsoft.com>

* add docstr

* reverse change in gitignore

* Update test/spark/custom_mylearner.py

Co-authored-by: Chi Wang <wang.chi@microsoft.com>

---------

Co-authored-by: Li Jiang <bnujli@gmail.com>
Co-authored-by: Chi Wang <wang.chi@microsoft.com>
sindlinger pushed a commit to Orgsindlinger/Auto-GPT-WebUI that referenced this pull request Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants