ghorpadeire opened a new pull request, #110:
URL: https://github.com/apache/fineract-loan-origination/pull/110

   Partially addresses #99.
   
   ## What this changes
   
   Three factual defects in existing Javadoc, each verified against the code:
   
   1. **`CreditScoreRepository`** documented a Redis service-layer cache. Redis 
does not appear anywhere else in the project — no dependency in `pom.xml`, no 
`@Cacheable`, no `CacheManager`. Removed the paragraph rather than leave a 
comment describing infrastructure that does not exist.
   
   2. **`ApplicantProfileRepository`** had two consecutive Javadoc blocks with 
no declaration between them. The block describing `findByApplication` was 
attached to `findAllByFineractClientIdAndTenantId`, and `findByApplication` 
itself was left undocumented. Moved the block onto the method it describes.
   
   3. **`ApprovalStageMismatchException`** stated that the global handler maps 
it to HTTP 409 Conflict. `GlobalExceptionHandler` returns 
`HttpStatus.FORBIDDEN` (403). Corrected the text to match the code.
   
   Also removed two paragraphs that restated the method signature, and 
tightened wording in the surrounding Javadoc. No behaviour change.
   
   ## A note on removing `<p>` tags
   
   #99 asks for HTML markup (`<p>`, `<ul>`, `<li>`) to be removed from Javadoc. 
That part does not look achievable while Spotless is enforced: 
google-java-format re-inserts `<p>` before every Javadoc paragraph.
   
   I removed all 207 line-leading `<p>` tags across `src/`, ran `./mvnw 
spotless:apply`, and they were restored — 13 of the 15 files I had changed 
reverted to their original content.
   
   Since CI runs `spotless:check`, a PR that strips those tags will either be 
undone by the formatter or fail the build. It may be worth revising that 
acceptance criterion in #99, or deciding whether the formatter configuration 
should change first. Happy to follow whichever direction you prefer.
   
   ## Verification
   
   ```
   ./mvnw spotless:apply     # no further changes
   ./mvnw spotless:check     # BUILD SUCCESS
   ./mvnw apache-rat:check   # BUILD SUCCESS
   ./mvnw test               # Tests run: 186, Failures: 0, Errors: 0, Skipped: 0
   ```
   
   I was not able to run `./mvnw verify` locally: the integration tests need 
Docker for Testcontainers and there is no container runtime on this machine. 
The change is comment-only so there is no behavioural risk, but I would rather 
flag that than imply I ran the full suite.
   
   ## Noticed while reading, left out of scope
   
   - `ApplicantProfileRepository` uses a fully-qualified `java.util.List` 
inline rather than an import. That is a code change rather than a comment 
change, so I left it. Happy to fix separately.
   - `CustomerCredentialRepository` and `StaffCredentialRepository` have no 
Javadoc at all. #99 explicitly excludes adding documentation where none exists, 
so they are untouched.


-- 
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]

Reply via email to