Skip to content
Bernd edited this page Mar 17, 2021 · 20 revisions

Conventions

General

  • The adopted style is 1TBS. We never omit braces even if there is only one statement.

  • All fields, parameters, and local variables which can be marked final should use this qualifier. This helps prevent accidental modifications, and any non-final variable might be seen as suspect or dangerous to manipulate as it may be modified asynchronously.

  • No class prefix (cg, cgeo) should be used.

  • Javadoc should be used on public methods. Undescribed parameters and return values should be omitted.

  • No raw threads should be created. cgeo heavily uses RxJava which provides nice ways of building asynchronous cancellable actions. The RxUtils class offers some additional utilities.

Imports order

Imports should either be untouched or put in this order for every Java file in the project:

cgeo.*

android.*

androidx.*

java.*

javax.*

everything else

Static imports will go under the non static imports within the blocks.

However, a file should not be modified solely for the purpose of fixing imports order.

Automated Code Quality Checks

Lint

Lint is the standard code quality check tool supported by Android Studio. Details can be found here.

The current Lint setting for c:geo can be found in file main/lint.xml.

Lint is not executed as part of the normal build. You can however start it locally as follows

  • Executing gradle target lintBasicDebug (either via Android Studio "Gradle" window in category "verification" or via command line)
  • This will create lint report files lint-results-basicDebug.html and lint-results-basicDebug.xml under main/build/reports
  • Open those reports to get an overview wrt found errors

Lint divides errors by severity in "Error", "Warning" and "Information". c:geo is configured such that the existence of lint errors with severity "Error" will fail the build on CI/Jenkins (see below).

However, at the time this was enforced, there were already around 80 "Error" level warnings active with c:geo. Those are listed in a file main/lint-baseline.xml and are automatically excluded from the server's decision to fail the build. It is the target of c:geo that this file becomes empty over time!

Should there ever be a need to recreate the file main/lint-baseline.xml, proceeed as follows:

  • Make sure your local build fulfills all relevant code quality criterias (e.g. clean, fresh master)
  • Run lint analysis e.g. using gradlew lintBasicDebug
  • take created xml report, remove all findings on severity "Warning" and replace main/lint-baseline.xml with this file

Checkstyle

Checkstyle rules of c:geo are defined in file checkstyle.xml (in root folder of c:geo). Checkstyle rules are enforced by CI/Codacy.

It is extremely recommended to install Checkstyle Plugin in Android Studio and run local checkstyle analysis before creating PRs, because it is very tedious to fix checkstyle findings found by CI/Codacy build (long roundtrip times). Checkstyle, among other things, checks also things like correct spacing/line break, parameter final keyword and import order.

CI: Codacy & Jenkins

c:geo uses Codacy as well as automated checks on Jenkins Builds to enforce certain code quality rules as follows:

  • Checkstyle rules as defined above (enforced via Codacy)
  • Lint rules as defined above. It is enforced that a build may not contain any Lint finding on Error level (enforced via Jenkins Build)
  • PMD rules as defined in Codacy (see Codacy c:geo settings)

It must be noted that Jenkins always apply rules on the whole build, while Codacy applies rules only on changed code files for each PR. Especially in combination with enforcement of stricter Codacy rules, this might sometimes lead to Codacy warnings on code which was not changed during the current PR.

Clone this wiki locally