Skip to content

Feature fortitude lint#31

Open
lamkina wants to merge 3 commits intomainfrom
feature-fortitude_lint
Open

Feature fortitude lint#31
lamkina wants to merge 3 commits intomainfrom
feature-fortitude_lint

Conversation

@lamkina
Copy link
Contributor

@lamkina lamkina commented Feb 5, 2026

This pull request introduces automated linting for Fortran code using Fortitude in the GitHub Actions workflow. It adds a reusable workflow configuration and a project-specific Fortitude configuration file to enforce code quality and style standards.

GitHub Actions workflow integration:

  • Added .github/workflows/fortitude.yaml to define a reusable workflow that runs Fortitude checks on Fortran code. The workflow supports configurable timeout and fail behavior, sets up Python, installs Fortitude, downloads a global config if a local one is missing, and runs the lint check with the appropriate exit code.

Fortitude configuration:

  • Added fortitude.toml to specify linting rules: sets line length to 120, selects core checks, ignores specific style rules, and excludes files matching certain patterns.

Expected time until merged

1-3 days

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

See this PR for testing logs.

Checklist

  • I have run ruff check and ruff format to make sure the Python code adheres to PEP-8 and is consistently formatted
  • I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@lamkina lamkina requested review from eirikurj and sseraj February 5, 2026 04:31
Copy link

@sseraj sseraj left a comment

Choose a reason for hiding this comment

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

One major problem is that Fortitude does not recognize PETSc syntax

src/adjoint/adjointUtils.F90:51:9: E001 Syntax error
   |
50 |           ! PETSc Matrix Variable
51 | /         Mat :: matrix
52 | |
53 | |         ! Input Variables
   | |__________________________^ E001

I am also seeing some false positives with E001 where the entire file is flagged.
This happens with src/adjoint/adjointExtra.F90 in ADflow, for example.

type: number
default: 10
description: "Timeout for the job in minutes"
FAIL:
Copy link

Choose a reason for hiding this comment

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

This should be an on/off flag instead?

"C132", # default-public-accessibility
"S201", # superfluous-implicit-none
"C141", # missing-exit-or-cycle-label
"C121" # use-all
Copy link

Choose a reason for hiding this comment

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

I think we should enable C121 so we have explicit use statements

# Core checks
select = ["E", "C", "OB", "MOD", "S", "FORT"]
# Style preferences
ignore = [
Copy link

Choose a reason for hiding this comment

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

Maybe ignore S102 as well?
S102 [*] need at least 2 spaces before inline comment
This is something that should be handled automatically by a formatter. fprettify added a rule for this recently, but it hasn't made it into a release yet.

On a related note, I think we will need to keep fprettify because Fortitude does not have the same formatting capabilities.

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.

3 participants