FINERACT-1956: Add Antora skeleton structure and consolidate contributor guides
This commit is contained in:
airajena
committed by
Aleksandar Vidakovic
Aleksandar Vidakovic
parent
3e98cf791a
commit
e4e0082872
@@ -0,0 +1,9 @@
|
||||
.Contributors Zone
|
||||
* xref:take-your-first-steps.adoc[Getting started]
|
||||
* xref:submitting-a-pull-request.adoc[Guidelines for submitting a PR]
|
||||
* xref:key-design-principles.adoc[Key Design principles]
|
||||
* xref:coding-conventions.adoc[Coding Conventions]
|
||||
* xref:checkstyle-and-spotless.adoc[Checkstyle and Spotless]
|
||||
* xref:error-handling.adoc[Error handling]
|
||||
* xref:logging-guidelines.adoc[Logging guidelines]
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
= Checkstyle and Spotless
|
||||
|
||||
The project enforces code conventions using **Checkstyle** and **Spotless**.
|
||||
|
||||
== Configuration Files
|
||||
* **Checkstyle**: `config/checkstyle/checkstyle.xml`
|
||||
* **Spotless**: `config/fineractdev-formatter.xml`
|
||||
|
||||
These tools run automatically during the Gradle build and will fail the build if violations are detected.
|
||||
|
||||
== Fixing Violations
|
||||
To automatically fix most formatting issues (Spotless and some Checkstyle), run:
|
||||
|
||||
[source,bash]
|
||||
----
|
||||
./gradlew spotlessApply
|
||||
----
|
||||
|
||||
To check for Spotless violations only:
|
||||
|
||||
[source,bash]
|
||||
----
|
||||
./gradlew spotlessCheck
|
||||
----
|
||||
@@ -0,0 +1,24 @@
|
||||
= Coding Conventions
|
||||
|
||||
== How We Code
|
||||
|
||||
We recommend that you configure your favourite Java IDE to match the project conventions.
|
||||
|
||||
=== Eclipse IDE
|
||||
For Eclipse, you can go to `Window > Java > Code Style` and import:
|
||||
* `config/fineractdev-formatter.xml` under the formatter section.
|
||||
* `config/fineractdev-cleanup.xml` under the cleanup section.
|
||||
|
||||
=== IntelliJ IDEA
|
||||
For IntelliJ, the project settings for code style should be automatically imported from the Gradle configuration.
|
||||
|
||||
== Formatting
|
||||
|
||||
We use Spotless to enforce code formatting. You can run the following command to automatically fix formatting violations:
|
||||
|
||||
[source,bash]
|
||||
----
|
||||
./gradlew spotlessApply
|
||||
----
|
||||
|
||||
This ensures consistent code style across the entire codebase.
|
||||
@@ -0,0 +1,22 @@
|
||||
= Error Handling Guidelines
|
||||
|
||||
== Exception Handling
|
||||
|
||||
* **Log or Rethrow**: When catching exceptions, either rethrow them or log them. Include the root cause.
|
||||
+
|
||||
[source,java]
|
||||
----
|
||||
catch (SomeException e) {
|
||||
throw new AnotherException("..details..", e);
|
||||
// OR
|
||||
LOG.error("...context...", e);
|
||||
}
|
||||
----
|
||||
|
||||
* **No Empty Catch Blocks**: Completely empty catch blocks are VERY suspicious. Do not "swallow" exceptions unless you are absolutely sure.
|
||||
* Such "normal exceptions" are often indications of using the wrong API (e.g., throwing an exception for an expected condition instead of returning empty/optional).
|
||||
|
||||
* **Tests**: In tests, typically propagate exceptions (`throws SomeException`) so the test fails if it occurs.
|
||||
* To test for an exception, use `Assert.assertThrows()`.
|
||||
|
||||
* **NullPointer**: Never catch `NullPointerException`. Fix the code instead.
|
||||
@@ -0,0 +1,54 @@
|
||||
= Key Design Principles
|
||||
|
||||
== RESTful API
|
||||
|
||||
The platform exposes all its functionality via a *practically-RESTful API*, that communicates using JSON.
|
||||
|
||||
We use the term *practically-RESTful* in order to make it clear we are not trying to be fully REST compliant but still maintain important RESTful attributes like:
|
||||
|
||||
* **Stateless**: The platform maintains no conversational or session-based state. This allows for horizontal scaling.
|
||||
* **Resource-oriented**: The API is focused around a set of resources using HTTP vocabulary and conventions (GET, PUT, POST, DELETE, HTTP status codes). This results in a simple and consistent API for clients.
|
||||
|
||||
See the online API Documentation for more details.
|
||||
|
||||
== Multi-tenanted
|
||||
|
||||
The Fineract platform has been developed with support for multi-tenancy at the core of its design. This means that it is just as easy to use the platform for Software-as-a-Service (SaaS) type offerings as it is for local installations.
|
||||
|
||||
The platform uses an approach that isolates an FI's data per database/schema.
|
||||
|
||||
== Extensible
|
||||
|
||||
While each tenant will have a set of core tables, the platform tables can be extended in different ways for each tenant through the use of Data tables functionality.
|
||||
|
||||
== Package Structure
|
||||
|
||||
The intention is for platform code to be packaged in a "vertical slice" way (as opposed to technical layers like controller/service/dao).
|
||||
|
||||
Source code location: `fineract-provider/src/main/java/org/apache/fineract`
|
||||
|
||||
Major modules include:
|
||||
* accounting
|
||||
* useradministration
|
||||
* infrastructure
|
||||
* portfolio
|
||||
** charge
|
||||
** client
|
||||
** fund
|
||||
** loanaccount
|
||||
|
||||
Within each vertical slice, there is a common packaging structure:
|
||||
* `api` - `XXXApiResource.java` (REST API implementation)
|
||||
* `handler` - `XXXCommandHandler.java` (Specific handlers)
|
||||
* `service` - Read + Write services for the functional area
|
||||
* `domain` - Object-Oriented concepts (Entities)
|
||||
* `data` - Data Transfer Objects (DTOs)
|
||||
* `serialization` - JSON conversion logic
|
||||
|
||||
== Maker-Checker (Four-Eyes Principle)
|
||||
|
||||
Enables applications to support a maker-checker style workflow process. Commands that pass validation will be persisted. Maker-checker can be enabled/disabled at a fine-grained level for any state-changing API.
|
||||
|
||||
== Fine-Grained Access Control
|
||||
|
||||
A fine-grained permission is associated with each API. Administrators have granular control over what roles or users have access to specific functions.
|
||||
@@ -0,0 +1,34 @@
|
||||
= Logging Guidelines
|
||||
|
||||
== Library
|
||||
We use http://www.slf4j.org[SLF4J] as our logging API.
|
||||
|
||||
* **Never use System.out/err**: Always use `LOG.info()` or `LOG.error()`.
|
||||
* **Use Placeholders**: Use parameterized logging instead of string concatenation.
|
||||
+
|
||||
[source,java]
|
||||
----
|
||||
// GOOD
|
||||
LOG.error("Could not... details: {}", something, exception);
|
||||
|
||||
// BAD
|
||||
LOG.error("Could not... details: " + something, exception);
|
||||
----
|
||||
|
||||
== Log Levels
|
||||
|
||||
* **`LOG.error()`**: Use for unexpected conditions that need operator attention.
|
||||
* Includes technical problems (database unreachable) and bugs.
|
||||
* Does NOT include validation errors (which should be user responses).
|
||||
* _Note_: SLF4J has no `FATAL` level; use `ERROR`.
|
||||
|
||||
* **`LOG.warn()`**: Use sparingly. Decide if it's an error or not.
|
||||
|
||||
* **`LOG.info()`**: Use for one-time actions (e.g., startup).
|
||||
* Do NOT use for regular application usage info (use Metrics instead).
|
||||
* Can be used freely in tests.
|
||||
|
||||
* **`LOG.debug()`**: Use for investigation data.
|
||||
* Not shown in default configuration but can be enabled for troubleshooting.
|
||||
|
||||
* **`LOG.trace()`**: Not used in Fineract.
|
||||
@@ -0,0 +1,62 @@
|
||||
== Submitting a Pull Request
|
||||
|
||||
We encourage all community members to contribute actively to the upstream Fineract codebase. All contributions come in the form of a pull request referencing a JIRA ticket outlining the intent of the contribution.
|
||||
|
||||
We request that your commit message include a FINERACT JIRA issue, recommended to be put in parentheses at the end of the first line. Start with an upper case imperative verb (not past form), and a short but concise clear description. (E.g. Add enforced HideUtilityClassConstructor checkstyle (FINERACT-821) or Fix inability to reschedule when interest accrued larger than EMI (FINERACT-1109) etc.).
|
||||
|
||||
If your PR is failing to pass our CI build due to a test failure, then:
|
||||
|
||||
** Understand if the failure is due to your PR or an unrelated unstable test.
|
||||
|
||||
** If you suspect it is because of a "flaky" test, and not due to a change in your PR, then please do not simply wait for an active maintainer to come and help you, but instead be a proactive contributor to the project - see next steps. Do understand that we may not review PRs that are not green - it is the contributor's (that's you!) responsability to get a proposed PR to pass the build, not primarily the maintainers.
|
||||
|
||||
** Search for the name of the failed test on https://issues.apache.org/jira/, e.g. for AccountingScenarioIntegrationTest you would find FINERACT-899.
|
||||
|
||||
** If you happen to read in such bugs that tests were just recently fixed, or ignored, then rebase your PR to pick up that change.
|
||||
|
||||
** If you find previous comments "proving" that the same test has arbitrarily failed in at least 3 past PRs, then please do yourself raise a small separate new PR proposing to add an @Disabled // TODO FINERACT-123 to the respective unstable test (e.g. #774) with the commit message mentioning said JIRA, as always. (Please do NOT just @Disabled any existing tests mixed in as part of your larger PR.)
|
||||
|
||||
** If there is no existing JIRA for the test, then first please evaluate whether the failure couldn't be a (perhaps strange) impact of the change you are proposing after all. If it's not, then please raise a new JIRA to document the suspected Flaky Test, and link it to FINERACT-850. This will allow the next person coming along hitting the same test failure to easily find it, and eventually propose to ignore the unstable test.
|
||||
|
||||
** Then (only) Close and Reopen your PR, which will cause a new build, to see if it passes.
|
||||
Of course, we very much appreciate you then jumping onto any such bugs and helping us figure out how to fix all ignored tests!
|
||||
|
||||
** Pull Request Size Limit documents that we cannot accept huge "code dump" Pull Requests, with some related suggestions.
|
||||
|
||||
Guideline for new Feature commits involving Refactoring:
|
||||
|
||||
[attributes]
|
||||
....
|
||||
If you are submitting PR for a new Feature, and it involves refactoring, try to differentiate "new Feature code" with "Refactored" by placing them in different commits. This helps review to review your code faster.
|
||||
....
|
||||
|
||||
|
||||
We have an automated Bot which marks pull requests as "stale" after a while, and ultimately automatically closes them.
|
||||
|
||||
|
||||
The steps below are part of our Github pull request template as a checklist while submitting. Don't treat this as mere boxes to tick off but rather ensure you're following each step so your PR doesn't get rejected and it can merged in a timely manner:
|
||||
|
||||
### Description
|
||||
|
||||
Describe the changes made and why they were made.
|
||||
|
||||
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
|
||||
|
||||
|
||||
### Checklist
|
||||
|
||||
Please make sure these boxes are checked before submitting your pull request - thanks!
|
||||
|
||||
- [ ] Write the commit message as described above
|
||||
- [ ] 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 as described xref:coding-conventions.adoc[here]
|
||||
|
||||
- [ ] 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.)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
== Take Your First Steps
|
||||
|
||||
=== Understand the basics
|
||||
Not sure what a pull request is, or how to submit one? Take a look at GitHub's excellent help documentation first.
|
||||
|
||||
=== Search Issues first; create an issue if necessary
|
||||
|
||||
Is there already an issue that addresses your concern? Do a bit of searching in our https://issues.apache.org/jira/projects/FINERACT/issues/FINERACT-1221?filter=allopenissues[issue tracker] to see if you can find something similar. If you do not find something similar, please create a new issue before submitting a pull request unless the change is truly trivial -- for example: typo fixes, removing compiler warnings, etc.
|
||||
|
||||
=== Discuss non-trivial contribution ideas with the community
|
||||
If you're considering anything more than correcting a typo or fixing a minor bug, please discuss it on the mailing list before submitting a pull request. We're happy to provide guidance, but please spend an hour or two researching the subject on your own.
|
||||
|
||||
== Create a Branch
|
||||
=== Branch from develop
|
||||
Develop currently represents work toward Apache Fineract 1.0.0. Please submit all pull requests there, even bug fixes and minor improvements.
|
||||
|
||||
[source]
|
||||
----
|
||||
git checkout develop
|
||||
----
|
||||
|
||||
=== Use short branch names
|
||||
Branches used when submitting pull requests should preferably be named according to issues prefixed FINERACT followed by a dash and the issue number, e.g. 'FINERACT-1234'. This is important, because branch names show up in the merge commits that result from accepting pull requests and should be as expressive and concise as possible.
|
||||
|
||||
[source]
|
||||
----
|
||||
git checkout -b [topic]
|
||||
----
|
||||
|
||||
=== Pull in changes
|
||||
Keep up to date
|
||||
|
||||
There will be development changes to both repositories almost every day for the foreseeable future so its important to keep your fork of the software up to date with the latest changes.
|
||||
|
||||
To pull in upstream changes from repository you forked:
|
||||
|
||||
[source]
|
||||
----
|
||||
git checkout develop
|
||||
git fetch upstream
|
||||
git merge upstream/develop
|
||||
----
|
||||
|
||||
WARNING: You should not have any local changes on your develop branch, if you do, use rebase!
|
||||
|
||||
|
||||
Once you have pulled all latest changes and your build was successful, push the just merged changes to the develop branch on your fork.
|
||||
|
||||
[source]
|
||||
----
|
||||
git push origin develop
|
||||
----
|
||||
=== Rebasing your work
|
||||
After you pulled all changes from upstream you need to rebase your topic branch with the latest code.
|
||||
|
||||
[source]
|
||||
----
|
||||
git checkout [topic]
|
||||
git rebase develop
|
||||
----
|
||||
|
||||
If you are ready push your topic branch to your GitHub account:
|
||||
[source]
|
||||
----
|
||||
git push origin [topic]
|
||||
----
|
||||
|
||||
Now thats your changes have been pushed to your github account, follow the steps and guidelines found xref:submitting-a-pull-request.adoc[here] to submit your pull request from your git account to fineract.
|
||||
Reference in New Issue
Block a user