-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create note module inside a new fineract-module directory (FINERACT-1932) #3349
Create note module inside a new fineract-module directory (FINERACT-1932) #3349
Conversation
@@ -0,0 +1,42 @@ | |||
.gradle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename the module to fineract-note
…dules FINERACT-1932: move note/data directory to the note module
f846b81
to
7877be9
Compare
'org.springframework.boot:spring-boot-autoconfigure:3.1.1', | ||
'org.mapstruct:mapstruct:1.5.5.Final', | ||
project(':fineract-core'), | ||
project(':fineract-provider'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module should not depend on the fineract-provider
module! Those references shall be either extracted into this module or the core module!
} | ||
|
||
rat { | ||
failOnError = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not turn off failing on error... if it fails, it is doing for a reason that we shall not ignore!
@@ -0,0 +1,255 @@ | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure you are moving these classes instead of just copying them!
'org.mapstruct:mapstruct:1.5.5.Final', | ||
project(':fineract-core'), | ||
project(':fineract-provider'), | ||
project(':fineract-loan') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module should not depend on the fineract-loan module! Rather the fineract-loan module should depend on this one (if applicable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments!
Abhinav and I spoke... probably better to abandon this PR. Actual code changes are a lot easier and less intrusive (easier to approve). |
Yes, I'll close this PR. The actual change is to not create new modules but to create custom AutoConfiguration classes in the existing modules |
Description
Ignore if these details are present on the associated Apache Fineract JIRA ticket.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Write the commit message as per https://github.com/apache/fineract/#pull-requests
Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
Create/update unit or integration tests for verifying the changes made.
Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.