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 check to CI #14

Merged
merged 3 commits into from
Nov 1, 2023
Merged

Add flake8 check to CI #14

merged 3 commits into from
Nov 1, 2023

Conversation

ChaoticRoman
Copy link
Owner

@ChaoticRoman ChaoticRoman commented Nov 1, 2023

Closes #10

Copy link

github-actions bot commented Nov 1, 2023

The code seems to be a GitHub workflow configuration file for running flake8, a Python linter, whenever a push is made to Python files in the repository.

Here are my observations:

  1. The workflow is correctly set to trigger on push events and specifically for Python files.

  2. The job is set to run on the latest version of Ubuntu which is good for most cases.

  3. The checkout and setup steps use the latest versions of their respective actions which is good practice to get the latest features and security updates.

  4. Python is being setup with version 3.11 which might be an issue if your project does not support that version. If your project supports a range of versions, it might be better to run the linter on the minimum supported version to catch any syntax or feature issues.

  5. The flake8 action you are using is suo/flake8-github-action. While this is not necessarily a problem, the action is not very popular and does not seem to be maintained. You might want to consider using a more popular and maintained action like github/super-linter which includes flake8.

  6. The GITHUB_TOKEN secret is correctly used to provide the action with access to the repository.

Overall, the workflow is well written but you might want to consider the Python version and the flake8 action you are using.

Copy link

github-actions bot commented Nov 1, 2023

The code changes are generally well made and follow good practices. Here are some specific feedback:

  1. You have added a new GitHub Action to run Flake8 linting on Python files. This is a great addition to ensure code quality and style consistency. Please make sure that the version of Flake8 you are using is compatible with the Python version you've specified (3.11).

  2. You've made changes to the code formatting to make it more PEP8 compliant, which is a good practice. However, you've added # noqa E501 to ignore line length in some places. It's generally better to refactor the code to fit within the line length limit. For example, you could assign the long string to a variable before using it.

  3. The import statement import readline # noqa F401 is marked to ignore unused import warning. If readline is indeed not used, it's better to remove the import statement. If it's required for some indirect reason, it would be helpful to add a comment explaining why.

  4. In the test.py file, you've made the indentation more consistent and improved the readability of the comments. This is a good change.

  5. In the core.py file, you've split a comment into two lines to fit within the line length limit. This is a good change, but again, try to avoid using # noqa: E501 where possible.

In general, the code changes are good and improve the readability and maintainability of the code. It's clear that you've put thought into following Python's style guide and best practices. Keep up the good work!

Copy link

github-actions bot commented Nov 1, 2023

The changes in this commit are well-structured and organized. Here are a few observations:

  1. Flake8 Workflow: The addition of a Flake8 workflow in the Github Actions is a good practice. It helps in maintaining the code quality by checking the Python code against some of the style guides in PEP 8.

  2. Python Version: You are setting up Python version 3.11 in the workflow. Make sure this version is compatible with your project requirements.

  3. Code Formatting: The changes in the cli.py, core.py, and test.py files mainly involve reformatting the code to make it more readable and to conform with PEP 8 style guide. This includes changes like adding line breaks to avoid lines that are too long and adding comments to explain the purpose of certain lines of code. These changes improve the readability of the code.

  4. Commented Code: In test.py, there are several lines of code that are commented out. If these lines are not needed, it's usually better to remove them to avoid confusion.

  5. Flake8 noqa: You have used # noqa: E501 to ignore line length limit in PEP 8. It's acceptable to use this when necessary, but it's generally better to try to reformat the line to fit within the length limit if possible.

  6. Secrets: You are using secrets.GITHUB_TOKEN in the workflow file. Make sure this secret is properly set in your repository settings.

Overall, this commit is well done, but you might want to consider the points mentioned above.

@ChaoticRoman ChaoticRoman merged commit bac9c20 into main Nov 1, 2023
2 checks passed
@ChaoticRoman ChaoticRoman deleted the flake8-ci branch September 12, 2024 22:49
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.

Liniting and style testing shall be part of CI
1 participant