IOhacker commented on PR #5279: URL: https://github.com/apache/fineract/pull/5279#issuecomment-3707073102
@Aman-Mital I don't support to keep a typo as backward compatibility. Several functional changes have been added or substracted in the APIs and they are informed in the change log in a release. I.e. you can see the email that I have sent around two weeks about the Loan Schedule Calculator which was a change with more impact than this typo. This is a non functional change. If people use this de branch must be aware that they using the unstable version and this change is very easy to fix, this is not a time consuming change. For functional changes that have an impact in DB stored values or change in data calculation we must add even a liquibase change in the persistente layer a new API or the backward compatibility in the controller layer. For me this change LGTM Regards Victor El sáb., 3 de enero de 2026 2:56 a. m., Aman-Mittal < ***@***.***> escribió: > ***@***.**** commented on this pull request. > ------------------------------ > > In > fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/api/LoanProductsApiResourceSwagger.java > <https://github.com/apache/fineract/pull/5279#discussion_r2658810769>: > > > @@ -1662,7 +1662,7 @@ private PutLoanProductsProductIdRequest() {} > @Schema(example = "FULL_LEAP_YEAR") > public String daysInYearCustomStrategy; > @Schema(example = "true") > - public Boolean allowPartialPeriodInterestCalcualtion; > + public Boolean allowPartialPeriodInterestCalculation; > @Schema(example = "179") > > Since this field appears to be part of the API surface, I think we should > retain tests for the legacy spelling alongside the corrected one to ensure > backward compatibility. > This would allow existing clients to continue working while validating the > new preferred field name. > If the old name is intentionally being dropped as a breaking change, that > should be explicitly documented. > > while tests run ok when fixing all typos, it will bound to break lot of > things outside > > I think backward compatibility must be added in this (As per Mail Thread). > > TLDR; I think Old Test cases must be retained and new tests must be added > for fixed typo, so that it can be checked for both scenarios. > > — > Reply to this email directly, view it on GitHub > <https://github.com/apache/fineract/pull/5279#pullrequestreview-3624265200>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ALD2ZARGKCWVOHW6DBSBOHL4E574HAVCNFSM6AAAAACQOHKF3CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMMRUGI3DKMRQGA> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> > -- 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]
