Skip to end of metadata
Go to start of metadata

You are viewing an old version of this page. View the current version.

Compare with Current View Page History

Version 1 Next »

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, like ViewSettingsTest
  • 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.

  • No labels