Aman-Mittal commented on PR #6271: URL: https://github.com/apache/fineract/pull/6271#issuecomment-5296803969
One follow-up to my review, and an amendment to it: in a couple of the inline comments I suggested recording the rationale on FINERACT-2684 — please disregard that part. My understanding is that FINERACT-2684 is scoped for small changes, and this PR is somewhat more than that. It establishes a repo-wide test execution convention (class-level `@Order` goes from 11 to 30 classes across three tiers, and `scripts/split-tests.sh` now depends on it for shard balancing), rolls `SavingsTestLifecycleExtension` out to roughly ten additional classes, and carries a behavioural fix to the `enable-auto-generated-external-id` handling in `LoanManualInterestRefundResponseStructureTest`. Would you be open to raising a dedicated ticket for this? It would give the tier definitions, the before/after timings, and the shard-balancing assumptions a durable home that future contributors can find — which is really what my inline questions were reaching for. If the split I mentioned in the review appeals, separate tickets for the savings cleanup rollout and the external-ID fix would work nicely too, but a single dedicated ticket covering this PR would already address my concern. Happy to defer to your judgement on the ticket layout — just flagging that FINERACT-2684 didn't look like the right long-term home for it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
