Skip to content

FINERACT-2811: Remove Cargo and Cargo plugin - #6410

Open
adamsaghy wants to merge 1 commit into
apache:developfrom
adamsaghy:FINERACT-2811
Open

FINERACT-2811: Remove Cargo and Cargo plugin#6410
adamsaghy wants to merge 1 commit into
apache:developfrom
adamsaghy:FINERACT-2811

Conversation

@adamsaghy

Copy link
Copy Markdown
Contributor

Cargo plugin is outdated and unmaintained. Cargo while it is maintained, i believe it would be better to leverage docker + compose or github Actions for testing.

Changes

  • Remove Cargo
  • Remove Cargo plugin
  • Add docker compose + github actions for CI testing
  • Add testing for building and deploying WAR (legacy reasons)

@adamsaghy adamsaghy reopened this Sep 7, 2026

@galovics galovics left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked the whole tree - the Cargo removal is clean, no dangling references anywhere (build.gradle files, workflows, CONTRIBUTING.md, adoc). -PcargoDisabled is gone from every workflow that used it, and the tomcat/driver Gradle configurations Cargo owned are removed without touching fineract-provider's own unrelated driver config. The WAR-in-Tomcat coverage trade-off is real (whole integration suite on a Tomcat-deployed WAR, running only post-merge on develop/release -> one @Smoke scenario, running on every PR) but it's the honest trade given the old coverage was already the only workflow not gated pre-merge, so it wasn't buying much. Good defensive touch on parsing the cucumber summary line to guard against "0 scenarios ran" silently passing.

One thing worth flagging clearly before merge: the tags = [x] -> tags = x fix (needed to make the new smoke job's tag filter actually work - the old Groovy list-coercion turned -Pcucumber.tags="@Foo" into the literal string "[@Foo]", which cucumber can't match against anything) has a side effect much bigger than this PR's stated scope. regression-safety-db-changes.yml passes tags the same way, and appears to have been passing green on every PR without ever actually executing its one gated scenario, because of the exact same bug. After this fix, that workflow starts running - and gating - for real. That's a valuable fix, but it's a significant, unrelated behavior change riding along in a Cargo-removal PR. Could you confirm run-regression-safety-db-changes is green here with a non-zero scenario count, add it a summary-line guard like the one you wrote for the smoke job, and call this out explicitly in the PR description? Whoever bisects a future regression-safety failure to this PR is going to have a bad time otherwise.

Two smaller things:

  • The JDWP debug port changes from 5000 to 5005 across several compose/env files - unrelated to Cargo removal, and it'll break every contributor's saved remote-debug run configuration with no changelog entry. Worth splitting out or at least calling out explicitly.
  • tomcat:10.1-jdk25-temurin is the only floating image tag in the new compose file while everything else in the repo (postgres, localstack, mock-oauth2-server) is pinned to an exact version - worth pinning the patch version too.

Recommendation: COMMENT

@IOhacker IOhacker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

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.

4 participants