IntroductionContents::sectnum::
This document is intended to serve as a pre-`submit` checklist for patch contributors and a pre-`commit` checklist for committers. Contributions following these guidelines have a much better chance of being accepted into Mifos.
Excellent Patches
- take `less than one hour` to look over and accept (not including build time)
- were created with "git format-patch" and apply cleanly using "git am"
- follow CodeSubmissionProcess, CodingStandards, and VettingPatches
- are easy to understand because they are accompanied by explanation/justification or the issue description is clear
If you know a patch is not excellent, reject it (see next section on how to do this). If you're not sure, ask for help on IRC and/or the mifos-developer or mifos-users mailing lists.
If the patch doesn't apply cleanly but is very small, manually apply it if you can do so quickly/easily.
First Pass
Initially, code should be scanned for more obvious mistakes. All unit, integration and acceptance tests pass without errors or failures. All code adheres to our standards
No "leftovers"
- No temporary debugging code exists.
- No commented-out code exists without explanations.
- No generated files.
Pretty
The following ensure readability and ease of merging.
- Code is consistently indented.
- Imports are organized.
- Unused imports are removed.
- Line endings are managed by the version control system
- This is generally not an issue with patches.
- Present whitespace-only and formatting changes in a separate patch from changes that actually affect code execution.
Unit tests
Unit tests are...
- suffixed with
Test
, likeViewSettingsTest
- new style (Cheetah/next generation code)
- written for TestNG
- old style (Rhino/legacy code, and Gazelle code for now)
- written for JUnit 4.
- part of a test suite, like AdminTestSuite or PPITestSuite.
- part of one of ApplicationTestSet1, 2, 3, or 4.
- part of ApplicationTestSuite.
i18n
- All strings are ready for translation.
- Translations are added to _is_IS.properties for pseudo L10n.
Licensing
All files use licensing information as specified at MifosLicensing.
Language
Proper grammar, spelling, and punctuation is used.
Ownership
Omit your own name. Ex: "this file created/mainted by me, Adam Monsen". Once the code is checked in, `we all own it, any one of us may maintain it`! Credit for contributors should appear in the AUTHORS file only.
This type of anonymity will:
- increase our bus-factor
- discourage territoriality
- promote group ownership and responsibility
Second Pass
When delving deeper into examining a patch, consider that more complex Mifos idioms are followed and look for deeper logic issues.
Intention-Driven Code
All method names and logic is self-explanatory or has Javadoc and/or comments.
Code Reuse
Efficient, sensible, and prolific code reuse is found.
Typesafe Containers
Generics are used whenever possible to parameterize containers.
Permissions
Pages and forms are accessible/usable only by appropriate users/roles.
Dates
Date-specific and timezone-specific code is not introduced.
Configuration Files
Configuration files should be found using ConfigurationLocator. Code touched which uses another method to locate configuration files should be refactored to use ConfigurationLocator.