-
Notifications
You must be signed in to change notification settings - Fork 15
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
Issue 0093 add lint at the actions #129
Conversation
I based the flake8 parameters and the lint.yml file configuration on the ones used by the Firedrake project (see Firedrake's setup.cfg for reference). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #129 +/- ##
==========================================
- Coverage 84.27% 84.23% -0.05%
==========================================
Files 56 56
Lines 3733 3729 -4
==========================================
- Hits 3146 3141 -5
- Misses 587 588 +1 ☔ View full report in Codecov by Sentry. |
@@ -763,4 +763,4 @@ def change_to_reference_hexa(p, cell_vertices): | |||
# pny = px * a21 + py * a22 + pz * a23 + a24 | |||
# pnz = px * a31 + py * a32 + pz * a33 + a34 | |||
|
|||
return (pnx, pny, pnz) | |||
# return (pnx, pny, pnz) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I think that removing the big block of comments is more appropriate to make the code easier to navigate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have temporarily removed this section in the latest commit. However, I plan on adding an improved version if we ever work with hexahedrals not based on extruded meshes. Thanks for the suggestion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. In this case you could have left the comment, but the flag that you added is even better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work setting up the linter in the CI system! My only recommendation is to avoid big blocks of comments in the future. As the code is under version control, it is always possible to check previous implementations without leaving them commented.
Description
This PR addresses Issue #93 by adding a linting step to the GitHub Actions workflow to ensure code quality and maintain consistency.
Changes Made
GitHub Actions Configuration (
lint.yml
):master
branch.Makefile Adjustments:
format
target to format the codebase usingautopep8
withsetup.cfg
as the configuration file.lint
target to runflake8
onsetup.py
,spyro/
, andtest/*.py
.Codebase Formatting:
Additional Notes
make lint
locally to catch issues before pushing to reduce CI errors.make format
) ensures code consistency based on the settings defined insetup.cfg
.