Skip to content

Remove fortitude from workflow #986

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 15, 2025
Merged

Remove fortitude from workflow #986

merged 3 commits into from
Aug 15, 2025

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Aug 15, 2025 •
edited by qodo-merge-pro bot
Loading

User description

deleted fortitude. pain to install for some users on macos

PR Type

Other

Description

  • Remove fortitude linting tool from project

  • Delete configuration file and workflow steps

  • Update Python dependencies in pyproject.toml

Diagram Walkthrough

flowchart LR
  A["fortitude tool"] --> B["configuration removal"]
  A --> C["workflow cleanup"]
  A --> D["dependency removal"]
Loading

File Walkthrough
Relevant files
Configuration changes
.fortitude.toml
Delete fortitude configuration file                                           

.fortitude.toml

  • Delete entire fortitude configuration file
  • Remove check settings and file extensions
 +0/-4     
lint-source.yml
Remove fortitude from CI workflow                                               

.github/workflows/lint-source.yml

  • Remove fortitude linting workflow steps
  • Delete source code checking commands
 +0/-6     
Dependencies
pyproject.toml
Remove fortitude dependency                                                           

toolchain/pyproject.toml

  • Remove fortitude-lint from dependencies list
 +0/-1     

Copilot AI review requested due to automatic review settings August 15, 2025 05:13
@sbryngelson sbryngelson requested a review from a team as a code owner August 15, 2025 05:14
Copilot AI reviewed Aug 15, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the fortitude-lint dependency and associated linting workflow due to installation difficulties on macOS. The change simplifies the toolchain by eliminating a problematic dependency while retaining other code quality checks.

Key changes:

  • Removed fortitude-lint from Python dependencies
  • Eliminated the fortitude linting step from the CI workflow
  • Deleted the fortitude configuration file

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
toolchain/pyproject.toml Removed fortitude-lint from the dependencies list
.github/workflows/lint-source.yml Removed the fortitude linting job from the CI workflow
.fortitude.toml Deleted the entire fortitude configuration file

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Aug 15, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missed Duplicate Check Removal

The workflow previously ran the fortitude check twice and enforced failure on findings; with fortitude removed, ensure an equivalent gate still exists (e.g., expand pylint/black checks or add alternative Fortran linting) so code quality doesn't silently regress.

 
- name: Looking for raw directives
  run: |
    ! grep -iR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="syscheck.fpp" ./src/*

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Aug 15, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Make directive check failure actionable

The raw directives check fails the job with a negated grep but provides no
output to help debugging. Add a fallback to print matches when found to make
failures actionable, and ensure grep returns zero when no files exist to avoid
false failures.

.github/workflows/lint-source.yml [31-33]

 - name: Initialize MFC
   run: ./mfc.sh init
 
 - name: Looking for raw directives
   run: |
-    ! grep -iR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="syscheck.fpp" ./src/*
+    set -euo pipefail
+    if matches="$(grep -IinR '!\$acc\|!\$omp' --exclude='parallel_macros.fpp' --exclude='syscheck.fpp' ./src/* || true)"; then
+      if [ -n "$matches" ]; then
+        echo "Found raw OpenACC/OpenMP directives in the following locations:"
+        echo "$matches"
+        exit 1
+      fi
+    fi
Suggestion importance[1-10]: 6

__

Why: The suggestion improves the robustness and debuggability of the CI script by handling grep errors and providing clearer output on failure, which is a valuable enhancement.

Low
  • More

@codecov
Copy link

codecov bot commented Aug 15, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.93%. Comparing base (f8830e5) to head (7007ee7).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #986   +/-   ##
=======================================
  Coverage   40.93%   40.93%           
=======================================
  Files          70       70           
  Lines       20288    20288           
  Branches     2517     2517           
=======================================
  Hits         8305     8305           
  Misses      10447    10447           
  Partials     1536     1536

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sbryngelson sbryngelson merged commit 88c0a11 into MFlowCode:master Aug 15, 2025
30 of 31 checks passed
@sbryngelson sbryngelson deleted the remove-fortitude branch September 2, 2025 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

Review effort 1/5

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sbryngelson