Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

 

Welcome contributors! We strive to include everyone's contributions. This page provides necessary guidelines on how to contribute effectively towards furthering the development and evolution of Sentry. You should also read the guide on setting up Development environment where you will find details on how to checkout, build and test Sentry.

Note: This guide applies to general contributors. If you are a committer, please read the How to commit as well.

...

Table of Contents
maxLevels3

...

What can be contributed?

There are many ways you can contribute towards the project. A few of these are:

Jump in on discussions: It is possible that someone initiates a thread on the mailing list describing a problem that you have dealt with in the past. You can help the project by chiming in on that thread and guiding that user to overcome or workaround that problem or limitation.

File Bugs: If you notice a problem and are sure it is a bug, then go ahead and file a JIRA. If however, you are not very sure that it is a bug, you should first confirm it by discussing it on the Mailing Lists.

Review Code: If you see that a JIRA ticket has a "Patch Available" status, go ahead and review it. It cannot be stressed enough that you must be kind in your review and explain the rationale for your feedback and suggestions. Also note that not all review feedback is accepted - often times it is a compromise between the contributor and reviewer. If you are happy with the change and do not spot any major issues, then +1 it. More information on this is available in the following sections.

Provide Patches: We encourage you to assign the relevant JIRA issue to yourself and supply a patch for it. The patch you provide can be codedocumentationbuild changes, or any combination of these. More information on this is available in the following sections.

Documentation: Contribute to the Sentry documentation by adding descriptions of new features, updating outdated documentation, and filling in documentation gaps. 

Providing Patches

In order to provide patches, follow these guidelines:

  • Make sure there is a JIRA:
    1. If you are working on fixing a problem that already has an associated JIRA, then go ahead and assign it to yourself.
    2. If it is already assigned to someone else, check with the current assignee before moving it over to your queue.
    3. If the current assignee has already worked out some part of the fix, suggest that you can take that change over from them and complete the remaining parts.
  • Attach the patches as you go through development:
    • While small fixes are easily done in a single patch, it is preferable that you attach patches to the JIRA as you go along. This serves as an early feedback mechanism where interested folks can look it over and suggest changes where necessary. It also ensures that if for some reason you are not able to find the time to complete the change, someone else can take up your initial patches and drive them to completion.
  • Test your changes before submitting a review:
    • Before you make the JIRA status as "Patch Available", please test your changes thoroughly. Try any new feature or fix out for yourself, and make sure that it works.
    • Make sure that all unit/integration tests are passing, and that the functionality you have worked on is tested through existing or new tests. JUnit is our test framework.

      Code Block
      mvn clean install -DskipTests (compile)
      
      mvn test (run all tests)

How to Contribute to Sentry

This page describes the mechanics of how to contribute new software to Sentry.

Getting the source code

Sentry uses git for version control.

Get the source code on your machine using Git. All development is done on branch "master".

Code Block

git remote add apache-sentry https://git-wip-us.apache.org/repos/asf/incubator-sentry.git
git fetch apache-sentry
git checkout -b local-master apache-sentry/master 

Making Changes

Before you start making changes, send an email to the dev@sentry.incubator.apache.org, or file a new issue using JIRA. Describe what your trying to do and how it will benefit the project.

Modify the source code in your favorite editor to add the new feature, enhancement or bug fix.

But take care about the following points

  • All public classes and methods should have informative Javadoc comments.
    • Do not use @author tags.
  • Contributions should pass existing unit tests.
  • New unit tests should be provided to demonstrate bugs and fixes. JUnit is our test framework:

Generating a patch

Unit Tests

Please make sure that all unit tests pass before posting your patch and that no new javac compiler warnings are introduced by your patch.

...

    • After a while, if you see

      Code Block

...

    • BUILD SUCCESSFUL
      

      all is ok, but if you see

      Code Block

...

    • BUILD FAILED
      

      then please examine error messages and fix things before proceeding.

...

  • How to create a patch

...

Check to see what files have been modified,

Code Block

git status

Add the files to the list of changes to be committed with:

Code Block

git add  <filename>
git add  <filename>

Commit the changes locally with:

Code Block

git commit -m "JIRA-#: JIRA title"

In order to create a patch, type:

...

  • file:
    • The preferred naming convention for Sentry patches is SENTRY-12345.patch, or SENTRY-12345.001.patch where 12345 is the JIRA number. You might want to name successive versions of the patch something like SENTRY-12345.002.patch, SENTRY-12345.003.patch, etc. as you iterate on your changes based on review feedback and re-submit them.
    • If the patch is targeted for other branch, the patch name should be followed by SENTRY-<JIRA number>.<patch version>-<branch name>, for example: SENTRY-12345.001-branch1.patch
    • The command to generate the patch is "git diff". Example:

      Code Block
      $ git diff > /path/to/SENTRY-1234.001.patch
       
      for Branch
      $ git diff > /path/to/SENTRY-1234.001-branch1.patch
    • Read the patch file. Make sure it includes ONLY the modifications required to fix a single issue.

  • Before you submit your patch:
    1. Your change should be well-formatted and readable. Please use two spaces for indentation (no tabs).
    2. Carefully consider whether you have handled all boundary conditions and have provided sufficiently defensive code where necessary.
    3. Add one or more unit tests, if your change is not covered by existing automated tests. Also add e2e tests if it is a new feature.
    4. Insert java docs and code comments where appropriate.
    5. If your patch implements a major feature or improvement, then you must fill in the Release Note field on the issue's Jira with an explanation of the feature that will be comprehensible by the end user.
    6. Please note that the attachment should be granted license to ASF for inclusion in ASF works (as per the Apache License §5).
    7. Please do not:

      • reformat code unrelated to the bug being fixed: formatting changes should be separate patches/commits.
      • comment out code that is now obsolete: just remove it.
      • make things public which are not required by end users.

      Please do:

      • comment code whose function or rationale is not obvious;
      • name the patch file after the JIRA – SENTRY-<JIRA#>.patch

...

  • Running RAT checks

    Before submitting your patch,

...

  • please run RAT check to make sure all the files have the necessary license headers.

    To do so

...

  • , run:

    Code Block

...

  • mvn 

...

  • verify -

...

  • DskipTests
    
  • How to apply someone else's patch file:
    • You can apply someone else's patch with the GNU patch tool. Example:

      Code Block
      $ cd ~/workspace/sentry # or wherever you keep the root of your Sentry source tree
      $ patch -p1 < SENTRY-1234.001.patch
      
    • Contributors may variously submit patches in a couple of different formats. If you get some dialog from the patch tool asking which file you want to patch, try variously the "-p1" or "-p0" flags to patch. Without any additional arguments, git diff generates patches that are applied using patch -p1. If you usegit diff --no-prefix to generate your patch, you have to apply it using patch -p0. The ReviewBoard tool understands both formats and is able to apply both types automatically.
  • Submitting your patch for review:
    1. To submit a patch, attach the patch file to the JIRA and change the status of the JIRA to "Patch Available".
    2. If the change is non-trivial, please also post it for review on the Review Board. Use the Repository "sentry" on Review Board.
    3. Use can use rbtools, a command line tool for Review board to speed up the process of submitting the review requests
      1. Download rbt

      2. Setup your ~/.reviewboardrc with the following content

        Code Block
        REVIEWBOARD_URL = "https://reviews.apache.org/"
        REPOSITORY = 'Sentry'
        TARGET_GROUPS = 'sentry'
      3. You can setup your username password like 

        Code Block
        rbt setup-repo --username=xx --password=xx
      4. Post the review: Commit your changes to your local repo and do:

        Code Block
        rbt post -g


      5. Fill in the Summary in the webpage and post it.
      6. Updating your review: append your changes the original commit and

        Code Block
        rbt post -r=<review#>
    4. Link the JIRA to the Review Board review and vise versa. JIRA has a feature you can use for this by going to More Actions > Link > Web Link when logged into JIRA.
  • Identify a reviewer:
    1. When posting on review board, always add the Group "Sentry" to the list of reviewers.
    2. Optionally, you may also add a specific reviewer to the review. You can pick any of the project committers for review. Note that identifying a reviewer does not stop others from reviewing your change. Be prepared for having your change reviewed by others at any time.
    3. If you have posted your change for review and no one has had a chance to review it yet, you can gently remind everyone by dropping a note on the developer mailing list with a link to the review.
  • Work with reviewers to get your change fleshed out:
    1. When your change is reviewed, please engage with the reviewer via JIRA or review board to get necessary clarifications and work out other details.
    2. The goal is to ensure that the final state of your change is acceptable to the reviewer so that they can +1 it.

Reviewing Code

Sentry uses the Apache Review Board for doing code reviews. In order for a change to be reviewed, it should be attached to the JIRA and posted on the review board. If the change is a minor change affecting only few lines and does not seem to impact main logic of the affected sources, it need not be posted on the review board. However, if the code change is large or otherwise impacting the core logic of the affected sources, it should be posted on the review board. Feel free to comment on the JIRA requesting the assignee to post the patch for review on review board.

Note: Not all patches attached to a JIRA are ready for review. Sometimes the patches are attached just to solicit early feedback regarding the implementation direction. Feel free to look it over and give your feedback in the JIRA as necessary. Patches are considered ready for review either when the patch has been posted on review board, or the JIRA status has been changed to 'Patch Available'. Find here a list of Sentry JIRAs marked Patch Available.

Goals for Code Reviews

The net outcome from the review should be the same - which is to ensure the following:

  • Bugs/Omissions/Regressions are caught before the change is committed to the source control.
  • The change is subjected to keeping the quality of code high so as to make the overall system sustainable. The implementation of the change should be easily readable, documented where necessary, and must favor simplicity of implementation.
  • Changes are evaluated from the perspective of a consumer (the reviewer) as opposed to the developer, which often brings out subtleties in the implementation that otherwise go unnoticed.
  • The change should be backward compatible and not require extensive work on existing installations in order for it to be consumed. There are exceptions to this in some cases like when work is done on a major release, but otherwise backward compatibility should be upheld at all times. If you are not clear, raise it is as a concern to be clarified during the review.

Code review guidelines

Following are some guidelines on how to do a code review. You may use any other approach instead as long as the above stated goals are met. That said, here is an approach that works fine generally:

  • Understand the problem being solved: This often requires going through the JIRA comments and/or mailing list threads where the discussion around the problem has happened in the past. Look for key aspects of the problem such as how it has impacted the users and what, if any, is the suggested way to solve it. You may not find enough information regarding the problem in some cases, in which case - feel free to ask for clarification from the developer contributing the change.
  • Think about how you would solve the problem: There are many ways to solve any code problem, with different ways having different merits. Before proceeding to review the change, think through how you would solve the problem if you were the one implementing the solution. Note the various aspects of the problem that your solution might have. Some such aspects to think about are - impact on backward compatibility, overall usability of the system, any impact on performance etc.
  • Evaluate the proposed change in contrast to your solution: Unless the change is obvious, it is likely that the implementation of the change you are reviewing is very different from the solution you would go for. Evaluate this change on the various aspects that you evaluated your solution on in the previous step. See how it measures up and give feedback where you think it could be improved.
  • Look for typical pitfalls: Read through the implementation to see if: it needs to be documented at places where the intention is not clear; if all the boundary conditions are being addressed; if the code is defensive enough; if any bad idioms have leaked in such as double check locking etc. In short, check for things that a developer is likely to miss in their own code which are otherwise obvious to someone trying to read and understand the code.
  • See if the change is complete: Check if the change is such that it affects the user interface. If it does, then the documentation should likely be updated. What about testing - does it have enough test coverage or not? What about other aspects like license headers, copyright statements etc. How about checkstyle and findbugs - did they generate new warnings? How about compiler warnings?
  • Test the change: It is very easy to test the change if you have the development environment setup. Run as many tests as you want with the patch. Manually test the change for functionality that you think is not fully covered via the associated tests. If you find a problem, report it.

How to give feedback

Once you have collected your comments/concerns/feedback you need to send it to back to the contributor. In doing so, please be as courteous as possible and ensure the following:

  • Your feedback should be clear and actionable. Giving subjective/vague feedback does not add any value or facilitate a constructive dialog.
  • Where possible, suggest how your concern can be addressed. For example if your testing revealed that a certain use-case is not satisfied, it is acceptable to state that as is, but it would be even better if you could suggest how the developer can address it. Present your suggestion as a possible solution rather than thesolution.
  • If you do not understand part of the change, or for some reason were not able to review part of the change, state it explicitly so as to encourage other reviewers to jump in and help.

Once you have provided your feedback, wait for the developer to respond. It is possible that the developer may need further clarification on your feedback, in which case you should promptly provide it where necessary. In general, the dialog between the reviewer and developer should lead to finding a reasonable middle ground where key concerns are satisfied and the goals of the review have been met.

If a change has met all your criteria for review, please +1 the change to indicate that you are happy with it.

Contributing to the Documentation

Before you edit the Sentry documentation, create a JIRA with the Docs component to track documentation changes. Request access to this wiki from the Sentry PMC and make the required changes. There is no formal review process for the wiki, but it is always a good idea to have the documentation reviewed before closing the JIRA. 

At the end, you should get a message on your console that indicates success.

Some things to note:

  • you may need to explicitly set MAVEN_HOME.
  • you may need to explicitly set JAVA_HOME.

Applying a patch

To apply a patch either you generated or found from JIRA, you can issue

Code Block

patch -p0 < SENTRY-<JIRA#>.patch

if you just want to check whether the patch applies you can run patch with --dry-run option

Code Block

patch -p0 --dry-run < SENTRY-<JIRA#>.patch

Contributing your work

Finally, patches should be attached to an issue report in Jira via the Attach File link on the issue's Jira. Please add a comment that asks for a code review. Please note that the attachment should be granted license to ASF for inclusion in ASF works (as per the Apache License §5).

When you believe that your patch is ready to be committed, select the Submit Patch link on the issue's Jira.

If your patch implements a major feature or improvement, then you must fill in the Release Note field on the issue's Jira with an explanation of the feature that will be comprehensible by the end user.

Please be patient. Committers are busy people too. If no one responds to your patch after a few days, please make friendly reminders. Please incorporate other's suggestions into your patch if you think they're reasonable. Finally, remember that even a patch that is not committed is useful to the community.

In some cases a patch may need to be updated based on review comments. In this case the updated patch should be re-attached to the Jira with the same name. Jira will archive the older version of the patch and make the new patch the active patch. This will enable a history of patches on the Jira. As stated above patch naming is generally SENTRY-#.patch where SENTRY-# is the id of the Jira.

Jira Guidelines

Please comment on issues in Jira, making their concerns known. Please also vote for issues that are a high priority for you.

Stay involved

Contributors should join the Sentry mailing lists. In particular, the commits@sentry.incubator.apache.org list (to see changes as they are made), the dev@sentry.incubator.apache.org list (to join discussions of changes).


See Also