Development Guidelines

IMPORTANT: Zanata's wiki has moved to https://github.com/zanata/zanata-platform/wiki

New link: https://github.com/zanata/zanata-platform/wiki/Development-Guidelines

---

(DRAFT)

These guidelines should be kept in mind when developing Zanata, and when reviewing changes (pull requests) for Zanata.

Review Considerations for Pull Requests

• Test coverage
• Static analysis, eg FindBugs
• Correctness
• Is the code correct? How do we know? Are the tests adequate?
• Clarity
• Is the code readable and understandable?
• Maintainability
• Will this make maintenance harder than it should be?
• Performance and scalability
• Will this perform well when N is large? (Where N is number of users, or projects, or documents, or strings, or requests per minute, etc.)
• Will this change cause existing operations to become significantly slower?
• Cluster safety
• If application is running in a cluster, how will other nodes see this data?
• Everything in the session should be Serializable
• Security
• Thread safety
• Watch out for shared mutable data

General

• Consider the code reviewer
  • Don't make trivial changes which make long diffs but don't improve anything
  • Don't reformat for the sake of it
    • If you need to reformat, use a separate commit
  • Don't amend long commits which may have been reviewed already - just add another commit
• Consider warnings from tools like FindBugs, but don't just do everything they say
• Prefer immutable data where possible: (We may need to choose a good functional programming library. Functional Java?)
• Make sure your pull request merges cleanly:
  • This allows QA to test the merged version.
  • If there are merge conflicts, the dev is usually the best one to fix them.
• Consider technical debt - are you reducing it or increasing it?
• Consider performance and scalability in APIs, in designs and in coding
  • Are you sure it will fit in memory? Can you stream it?
  • Will this enhancement incur performance costs for all requests, even though only some requests will benefit?
  • Don't use exponential algorithms
    • Try to minimise iteration - better data structures may help (eg look up in a map instead of iterating over a list)
• Use static analysis; eg keep FindBugs clean!
• If rewriting something (code, pages, docs, tests), check the old version so that we don't "forget about" existing features, docs or tests
• If you need to download a build dependency (eg an app server), make sure you cache it outside the build directory (eg ~/.m2 or ~/Downloads, not ./target) so that mvn clean won't destroy it, but be careful how you cache it to avoid staleness.
• Don't use enums simply to hold string constants - use a class with public static final fields!

Seam-specific

• Never inject lower-scoped beans into larger-scoped beans
  • eg a Session bean into an Application bean
  • this can lead to thread safety problems

CDI (read the weld manual)

• Don't use @Singleton: use @ApplicationScoped
• If you miss flash scope, use a conversation or other DeltaSpike scope
• @SessionScoped beans should generally be @Synchronized for thread safety
• Tip: Code which uses database needs @Transactional
• Tip: Event handlers with "@Observes(during=TransactionPhase.AFTER_SUCCESS)" will need @Async if they use @Transactional

Docs

• Update docs if appropriate
• Remember to update the release notes for infrastructure changes (Straightforward JIRA issues will be listed as one-liners during the release process)

Database

• Use named queries
• Use one operation per liquibase changeset
• For long operations, check precondition (eg index does not exist) first
  • This will help sysadmins, in case the operation times out

Logging

• Don't use System.out/err.println - generally prefer log.debug
• Learn to configure the logging framework, eg to enable DEBUG

Exception handling

• Never catch and ignore, eg try { something(); } catch (Exception e) {/* VERY BAD*/}
• Catch and handle, or throw another (chained) exception
• Don't log an ERROR and throw too
  • logging at DEBUG is okay
  • logging as WARN may be used if you suspect third party code will catch the exception without logging)

JavaScript code

• Don't paste .js into our source tree; use webjars.org or similar approach to package .js as a Maven dependency

UI code

• test UI changes on both Firefox and Chrome
• all UI code must be internationalised, with Jenkins jobs to:
  • extract POT, upload to translate.zanata.org
  • download PO from translate.zanata.org, commit to source tree

Tests are not optional

• Unit tests are preferred in most cases
  • otherwise, write a system test
• Especially important for dynamically typed code, including JavaScript and Groovy

Mock testing

• If you need a dozen mocks, something is wrong with your test or with the code - refactor!
• spy(objectUnderTest) should never be needed: refactor!
• Prefer when(mock.methodCall()).thenReturn(result), not doReturn(result).when(mock).methodCall()

Selenium Tests

• Don't use implicit wait
• Don't even use explicit "wait for condition" if you can help it: "timed out" errors aren't helpful
• Try to use waitForPageSilence(), then just assert the condition you want to check
  • This should lead to better error messages
  • Consider improving waitForPageSilence() if you need it

Keep Zanata server together

If some code is deployed with Zanata server and maintained by Zanata team:

• it should not be an external dependency!
• the code should be in the same repo, tagged together, tested together
• it should be built from source into zanata.war automatically (eg by Maven)
  • this makes integration and testing far easier
  • important for continuous integration and continuous delivery

New technologies and dependencies

Don't introduce major new technologies without considering:

• reliability
• cross-platform support (RHEL, Fedora, Mac, Windows)
• compatibility with other technology choices (or future choices)
  • eg Lombok vs IDEs
  • eg Lombok vs Java 8, 9...
• how to build and release (currently via Maven)
  • this includes Jenkins setup - if you break it, you bought it
• how to deploy (currently via MEAD)
• i18n (for UI technologies)
• security
• unit testing and system testing
• maintainability of code
• consider the "hit by a bus" factor
• the rest of the team needs to be able to work with the code

Third party libraries

• Consider parentage/governance (will it be maintained long term?)
• Check for compatible licence
• Check code quality, tests, frequency of release (will it keep up with new tech, eg Java 9 bytecode?)
  • warning signs: ignored pull requests, ignored bugs
• Be especially careful with libs which manipulate code/bytecode
• Be prepared to keep the library up to date
• For client-side code: is the dependency in Fedora, and up to date, or will we need to add it ourselves?