As an open source project, Metron welcomes contributions of all forms. The sections below will help you get started.

Getting started - Basic Stuff

• First rule of Metron is there is no Metron...
• Avoid cryptic abbreviations. Single letter variable names are fine in very short methods with few variables, otherwise make them informative.
• Clear code is preferable to comments. When possible make your naming so good you don't need comments. When that isn't possible comments should be thought of as mandatory, write them to beread.
• Logging and configuration should be consistent and usable.
• There is not a maximum line length (certainly not 80 characters, we don't work on punch cards any more), but be reasonable.
• Don't be sloppy. Don't check in commented out code: we use version control, it is still there in the history. Don't leave TODOs in the code or FIXMEs if you can help it. Don't leave println statements in the code. Hopefully this is all obvious.
• We want people to use our stuff, which means we need clear, correct documentation. Along with tests, documentation should be considered a part of any feature.
• Don't duplicate code (duh).
• Need to discuss Code Style Guidelines

How To Contribute

We are always very happy to have contributions, whether for trivial cleanups, little additions or big new features.

If you don't know Java or Scala you can still contribute to the project. We strongly value documentation and gladly accept improvements to the documentation.

Contributing A Code Change

To submit a change for inclusion, please do the following:

• If the change is non-trivial please include some unit tests that cover the new functionality.
• If you are introducing a completely new feature or API it is a good idea to start a discussion and get consensus on the basic design first.  Larger changes should be discussed on Slack before submission.
• New features and significant bug fixes should be documented in the Github Issues section
• Make sure you have observed the recommendations in the code style guide.
• Note that if the change is related to user-facing protocols / interface / configs, etc, you need to make the corresponding change on the documentation as well.
• It is our job to follow up on patches in a timely fashion.
• Always make a new branch for your work.
• Patches should be small to facilitate easier review. Studies have shown that review quality falls off as patch size grows. Sometimes this will result in many small PRs to land a single large feature.

Reviewing and merging patches

Everyone is encouraged to review open pull requests. We only ask that you try and think carefully, ask questions and are excellent to one another. Code review is our opportunity to share knowledge, design ideas and make friends.

When reviewing a patch try to keep each of these concepts in mind:

Architecture

• Is the proposed change being made in the correct place? Is it a fix in a backend when it should be in the primitives?  In Kafka vs Storm?
• What is the change being proposed?  Is it based on Community recognized issues?
• Do we want this feature or is the bug they’re fixing really a bug?
• Does the change do what the author claims?
• Are there sufficient tests?
• Has it been documented?
• Will this change introduce new bugs?

Intent

Implementation

Grammar and style

These are small things that are not caught by the automated style checkers.

• Does a variable need a better name?
• Should this be a keyword argument?

Merge requirements

Because Metron is so complex, and the implications of getting it wrong so devastating, Metron has a strict merge policy for committers:

• Patches must never be pushed directly to master, all changes (even the most trivial typo fixes!) must be submitted as a pull request.
• A committer may never merge their own pull request, a second party must merge their changes after it has be properly reviewed. There should be 2 parties besides the committer that have reviewed the patch before merge.
• A patch that breaks tests, or introduces regressions by changing or removing existing tests should not be merged. Tests must always be passing on master. This implies that the tests have been run...
• If somehow the tests get into a failing state on master(such as by a backwards incompatible release of a dependency) no pull requests may be merged until this is rectified.
• All merged patches must have 100% test coverage.

The purpose of these policies is to minimize the chances we merge a change that jeopardizes has unintended concequences.