Contributor Guidelines

Contributing Documentation Changes

You were confused about our documentation? You ran into a pitfall that others also might run into? Help us making the Marathon documentation great.

The documentation that is published here actually gets generated from what is found in the docs directory.

If you simply want to correct a spelling mistake or improve the wording of a sentence, you can browse the mark down files here and use the edit button above the markup. That will make it easy to create a pull request that will be reviewed by us.

If want to contribute a larger improvement to our documentation:

Edit the files in the docs directory.
Check the rendered result as described in the docs/README.md.
If you are feeling perfectionistic, check if there is already an issue about this documentation deficiency here. Prefix your commit message with with “Fixes #1234 - “ where #1234 is the number of the issue.
Create a pull request against master.

Please rebase your pull requests on top of the current master using git fetch origin && git rebase origin/master, and squash your changes to a single commit as described here. Yes, we want you to rewrite history - the branch on which you are implementing your changes is only meant for this pull request. You can either rebase before or after you squash your commits, depending on how you’d like to resolve potential merge conflicts. The idea behind is that we don’t want an arbitrary number of commits for one pull request, but exactly one commit. This commit should be easy to merge onto master, therefore we ask you to rebase to master.

After you pull request has been accepted, there is still some manual work that we need to do to publish that documentation. But don’t worry, we will do that for you.

Getting Started with Code Changes

Maybe you already have a bugfix or enhancement in mind. If not, there are a number of relatively approachable issues with the label “good first issue”.

Submitting Code Changes to Marathon

A GitHub pull request is the preferred way of submitting patch sets. Please rebase your pull requests on top of the current master using git fetch origin && git rebase origin/master, and squash your changes to a single commit as described here. Yes, we want you to rewrite history - the branch on which you are implementing your changes is only meant for this pull request. You can either rebase before or after you squash your commits, depending on how you’d like to resolve potential merge conflicts. The idea behind is that we don’t want an arbitrary number of commits for one pull request, but exactly one commit. This commit should be easy to merge onto master, therefore we ask you to rebase to master.
Please start your commit message with “Fixes #1234 - “ where #1234 is the github issue number that your pull request relates to. Github will automatically link this PR to the issue and make it more visible to others.
Any changes in the public API or behavior must be reflected in the project documentation.
Any changes in the public API or behavior must be reflected in the changelog.md.
Pull requests should include appropriate additions to the unit test suite.
If the change is a bugfix, then the added tests must fail without the patch as a safeguard against future regressions.
Run all tests via the supplied ./bin/run-tests.sh script (requires docker).

Test Guidelines

General guidelines

Tests should extend mesosphere.UnitTest
Tests should avoid testing more behavior than necessary; IE - don’t test other libs or services if a mock/stub suffices.

Fixtures

Our testing guidelines regarding fixtures are as follows:

DO NOT use var thing: Type = _ with before / after.
Fixture instantiation and tear down should be defined and handled in the same module.
When testing actor systems, we instantiate an actor system for the entire suite. It’s acceptable to let the actors get cleaned up when the suite finishes. Use either unique actor names or use the loan-fixtures technique to get a unique ActorRefFactory per test.
When teardown-per-test is desired, use the loan-fixtures methods. Otherwise, prefer parameterized case classes or traits.

Source Files

Public classes should be defined in a file of the same name, except for trivial subtypes of a sealed trait. In that case, the file name must be the plural form of the trait, and it must begin with a lowercase letter.
Some of the source files have diagrams as part of the comments using the PlantUML language.
To see a rendered diagram use the online version or install locally.

Style

Style Checker

Executing the test task in SBT also invokes the style checker. Some basic style issues will cause the build to fail: While you should fix all of these, you can disable them if it is a false positive with // linter:ignore Arg as listed here: Linter.

Type Annotations

Scala has a powerful type inference engine. The reason for including more type annotations than are required to make the program compile is to increase readability.

Methods with public or protected scope must have an explicit return type annotation. Without it, an implementaiton change later on could accidentally change the public API if a new type is inferred.
Nontrivial expressions must be explicitly annotated. Of course, “nontrivial” is subjective. As a rule of thumb, if an expression contains more than one higher-order function, annotate the result type and/or consider breaking the expression down into simpler intermediate values.

Null

Assigning the null value should be avoided. It’s usually only necessary when calling into a Java library that ascribes special semantics to null. Prefer scala.Option in all other cases.

Higher Order Functions

GOOD:

xs.map(f)

xs.map(_.size)

xs.map(_ * 2)

xs.map { item =>
  item -> f(item)
}

xs.map {
  case a: A if a.isEmpty => f(a)
  case a: A              => g(a)
  case b: B              => h(b)
  case other: _          => e(other)
}

Note: match expressions must be exhaustive unless the higher-order function signature explicitly expects a PartialFunction[_, _] as in collect, and collectFirst.

for {
  x <- xs
  y <- x
  z <- y
} yield z

BAD:

xs map f // dangerous if more infix operators follow

xs.map(item => f(item)) // use curlies

xs.flatMap(_.flatMap(_.map(_.z))) // replace with a for comprehension