Skip to content

Creating a pull request

Oskar W edited this page Mar 25, 2021 · 9 revisions

FAQ

  • I created a PR and want to get it reviewed, what should I do?

    • Please leave a comment with your PR at the referring Jira ticket and ask for a review.
  • I wrote a comment on the Jira ticket but nobody reviewed my ticket yet, what should I do?

    • Sometimes it can take a few days for one of our members with writing access to see the comment, please be patient and do not ping any members in the comments.

Before you create a pull request

  • Run the Android Studio Formatter on your changes only. More information can be found here
  • Use the Gradle task check to fix issues concerning your changes
  • Use the Android Studio code inspection tool to fix issues concerning your changes
  • Take a look at our commit message guidelines and previous pull requests to see our conventions for commit messages and pull request titles. The most important things are:
    • the commit should be named after the ticket number in Jira (e.g. CATROID-509) and be followed by a few words that describe the changes
    • Make sure that there is only 1 commit in the PR
    • The branch should be named according to the Jira ticket (e.g. CATROID-509)

After creating a pull request

After creating you can see your pull request on Github with some additional information. Make sure to insert all requested information into the PR comment just like the reference to the corresponding Jira ticket as well as a short description of what you have done.

Selfreview

Every PR has a checklist you should work through, which looks like this:
pullrequestchecklist
Make sure to work through all the points and check them before requesting a code review.

Conflicts

While working through the list you should check for any conflicts that may occur while merging your PR into our develop branch. This would look something like this;
conflicting files
In this case, try to rebase your branch onto the current upstream/develop and resolve the conflicts. In this case, we can see that there is a conflict in CategoryListFragment.java which should be resolved.

If there are no conflicts or we fixed them, we continue by waiting for Jenkins to finish all checks!

Checks beforehand


Pull Request
Don´t worry if any checks fail, just get into the Details.

Jenkins

This should lead you to the Jenkinsrunner of your PR and look similar to this:
Jenkins-landingpage
The Jenkinsrunner gives you a whole lot of information about the Build of your PR, but there are two main things you should have a look at before requesting a code review:

  • Static Analysis
  • Test Result

Static Analysis

Take a look at the Static Analysis and Fix all warnings on Jenkins so it looks like this:
Static_Analysis_Jenkins
The static analysis tool basically checks your code against any coding standard issues and so on. So this does not report any functionality errors but this ensures that there is some structure in the written code. You can open them, by clicking on the warnings link. Afterward, you can dig deeper into Checkstyle and PMD, which are your coding style issues. You can just follow the links to each issue. To fix the warnings just click through them and fix them in the code accordingly.

Test Results

The test results show you if any of our test cases failed for your build. This part looks at the functionality of your changes. Please make sure that there are no failing tests that are associated with your changes! Fix failing tests on your PR and do not forget to squash any additional commits to one. You can also use git commit --amend to add your changes.
testfails
In the uncommon case that there are failing tests that are not related to your changes, try to run them locally. If the tests pass locally but not on the Jenkins-run, just add a note to the PR in order for the reviewer to notice that issue.

Request code review

In order to get your PR quickly merged let a team member with writing access review your code and join the discussion
After the code review of a team member, your PR will be reviewed by the PO. If any changes are requested, please change them and redo the whole procedure.

Repeat until merged