— Colm Carew 2018/05/11 03:55

A presentation of this process as well as a guide can be found here : https://docs.google.com/presentation/d/17kGNWKLKxOgc6KfrSidwojwKGvvpRA-2Am3dDusX-PY/edit#slide=id.p4

The initial review (credits to Anna!) - https://docs.google.com/document/d/1MxZtlwdKw69Y8HNs_1JxBMuFI97N-zsvnEtHRWJOfnw/edit

Note : It is the responsibility of the person who opened the PR to merge it, not the person who approves it. Though there are circumstances where the approver/anyone can merge it in.

Feature branches should include the name (IDMS-1235) of the ticket (or one of if many tickets) at the start of the branch name such as:

IDMS-1234-description

Every commit message should also contain the ticket, example commit message:

IDMS-1234 : Setting up a git demo

• Checkout the main/rel branch and pull the latest changes
• Checkout your branch
• Merge the base branch into your current branch - this will spot any merge conflicts (if any) and you should resolve locally via Intellij Merge features
• Push your branch
• Before creating a PR, ensure that you have a stable build on your branch.
• Log into Bitbucket in a browser and go to the project you have been working on, click pull requests and click open new pull request - (you can do this via the command line but it would be better to do everything through Bitbucket where possible)
• It should automatically populate if you just pushed your branch, if not you want to merge your branch into the main/rel branch
• Ensure to check Close Branch. This will close the branch and delete its Jenkins build once merged.
• Once pull request is open you will need to wait for someone to approve it - if it is important ask someone to review it if it is urgent or your notice it should should have been approved by now
• Once approved you can merge (ensure you squash your commits) your branch into the main/rel branch
• Ensure that you commit to the correct destination(s), for example if there is a branch for the current release, the merge should be to this and to main

This is just a guideline, common sense should be used when reviewing PR's

• Ensure code readability / commenting where necessary / javadoc.
• Good design / Security considerations.
• Inclusion of flyway scripts where necessary (e.g. new domains, updates to existing domains etc.)
• DO NOT MODIFY AN EXISTING FLWAY SCRIPT!
• Ensure integration / unit tests are present (depending on the complexity of the PR / Feature, more / less tests may be expected. This should be handled on a case by case basis.)
• Ensure any local files such as .idea etc are not committed by including them in the .gitignore.
• Check code formatting.
• Ensure conflicts are resolved.
• Ensure stable build is present on PR.
• Check the Sprint for the JIRA ticket(s) being merged - only the current sprint tickets should be merged to dev.

The names below should be asked for reviews first followed (any one of them, in no particular order). If none available most other mid to senior developers should be able to review.

• Snnp Manager - Won, Andrey, Anna, Shea
• Ticketer - Anna, Shea, Colm, Avi, Kevin
• Noc Portal - Won, Anna, Andrey
• Reporting Manager - Anna, Milos
• eBonding - Colm, Anna
• Soap - Colm
• Fiber - Milos, Avi, Colm, Kevin
• Content Distributor - Milos, Avi, Colm
• Right of Way - Anna
• Ansible - Milos, Anna, Avi, Colm
• Alarm Cache - Andrey, Won
• CAS - Anna, Avi, Milos, Andrey
• Common Header - Avi, Anna, Won
• ECI - Joe, Kevin