Ambika-Sony commented on PR #5677: URL: https://github.com/apache/fineract/pull/5677#issuecomment-4119916649
> Hi @Ambika-Sony, thanks for the note and for your work on this. I actually hadn’t noticed #5665 when I started #5677. I saw a couple of closed PR targeting [FINERACT-2494](https://issues.apache.org/jira/browse/FINERACT-2494?jql=project%20%3D%20%22Apache%20Fineract%22%20AND%20labels%20%3D%20gsoc-fineract-evidence%20ORDER%20BY%20createdDate), but I missed #5665, apologies for that. > > After comparing #5665 and #5677, my view is that #5677 is a bit cleaner for [FINERACT-2494](https://issues.apache.org/jira/browse/FINERACT-2494) because it keeps the change strictly tests-only, in line with the ticket scope. In #5665, there are also `ApiParameterHelper` production-code changes; those hardening updates are valuable, but they may be better handled in a separate PR/ticket so scope and review impact stay clear. > > I’m happy to align and avoid duplication. If maintainers prefer a single path, I can either: > > 1. close this PR, or > 2. keep this one for tests-only and leave behavior changes to a follow-up PR. > > Happy to follow maintainer guidance either way. Hi @San-43, Regarding the scope: the 'production-code changes' in #5665 weren't just extra additions; they are hardening fixes specifically required to address NullPointerExceptions and boundary errors that my 22+ test cases exposed. A tests-only PR (#5677) would pass on the current code but would leave these underlying stability issues unaddressed. I believe it's critical for the project to not only have tests but to resolve the vulnerabilities those tests reveal. I'm happy to let the maintainers decide the best path forward, but I will be keeping #5665 active as a comprehensive solution for FINERACT-2494 that ensures both coverage and core stability. -- 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]
