# Git

### Who is in charge here?

In the future DevOps should NOT be responsible for merging all tickets, that would ideally be the reposibility of


DevOps will still be available to help with issues that arise as well as to release the changes, but placing us at the centre of the git lifecycle creates a choke point as well as uses a lot of our time.

### Branching
As a rule we utilise [git flow](https://danielkummer.github.io/git-flow-cheatsheet/) for development. 90% of this you don't need to know, so I've condensed what I see as the important bits

A few rules:-
1. Don’t work in develop or master
- They're protected so you can't push to them anyway
2. Always branch from develop
- No more mega branches
3. Commit and push regularly
4. Keep your local repo up to date with the remote and vice versa
5. Rebase wherever possible
- More on this later
6. Don't reopen branches that have been merged to fix a bug
- create a new branch with the prefix `bugfix`

Three types of branches:-
- `feature`:- Used to create a new feature or extend functionality
- `bugfix`:- Used to fix bugs found in testing
- `hotfix`:- Used to fix bugs that are already in prod
- n.b. this is merged directly into master and so does not form
part of the standard 'rapha release process'

### Commit Messages

Mix and match of the best git commit messaging standards.
- [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0-beta.4/#summary)
- [Commit message guidelines](https://gist.github.com/robertpainsi/b632364184e70900af4ab688decf6f53)

1. The summary line will be written in the imperative form
- 'fix bug' not 'fixed bug'
- 'add feature' not 'added feature'
2. Ideally the summary line should be 50 chars or less
3. All subsequent lines in the commit message are 72 characters or less
4. First line of the commit is a brief summary
5. The body will contain a more detailed explanation of the change.
**A blank line MUST seperate the summary from the body**
6. If a commit introduces a breaking change then the body will be
prefaced by `BREAKING CHANGE:`
- Can be used with **ANY** type of commit
7. The JIRA ticket id will be added as a new line with prefix
`ticket_id`
8. The summary will take the following form:-

<type> (scope) (!) : <description>

type = ['fix','feat','chore','style','refactor','improvement']
scope = what part of the codebase was changed

#### Commit message example

feat(raphafacades): add single purchase of product

BREAKING CHANGE: Add new `singlePurchaseProduct` field, add new solr 
index and ensure product can only be purchased once.

ticket_id: RAPHACC-007

#### Why this though?
- Entire summary visible when you run `git log --oneline`
- As it is standardised we can use common tools to create a changelog
- Helps to correctly version releases (MAJ.MIN.PATCH)

#### Tips
If using `vim` to edit commit messages, set the following in your
`.vimrc` file (usually in `~`):

set backspace=2
syntax on
autocmd Filetype gitcommit setlocal spell textwidth=72

### Pull Requests (PR's)

None of this please


Every single merge request needs to be peer reviewed and approved by at a bare minimum 1 colleague (ideally one that understands the changes that have been made).
At the minute it is required that you request a review manually, however in the long term I believe it will be beneficial to automate this (so people don't forget).

Please ensure that in the title of the pull request you put the JIRA ticket ID so that it can be linked back to the ticket.

#### Reviews and Approvals
If you are asked to review someone's PR, please ensure the following:-
1. Tests (if any) pass
2. The code changes abide by the releavnt coding standards
3. The code makes logical sense
4. The code is easy to read and understand
4. COMMENTS/documentation in someway of what has changed

Do not feel like you have to approve a ticket, even if there is a push for a release, if the code is not up to scratch you have every right to push the change back as this is how we reduce technical debt.

If your code is being reviewed and the change is rejected without valid reason, then escalate it to a relevant manager/DevOps and it will be reopened and reviewed again.
As well as this, don't take a PR rejection as a bad thing use it as an opportunity to improve the code you've written.

### Rebase vs Merge

When bringing the latest changes into your development/feature branch I personally believe it is preferable to rebase your branch against develop.

Rebasing, at its core, is simply applying the changes from the develop branch onto your branch *before* the changes that you have made. I won't go into the intricacies of how this work, mostly because I don't know the specifics, but the following diagrams explain it:-

In essence rebase will place the changes from develop **before** the changes that have been made in the feature branch, whereas a merge will smush them together making a somewhat messy commit history (as well as adding in additional commits).

#### How to rebase your feature branch

# get the latest changes from the remote
git checkout develop 
git pull 

# rebase your branch
git checkout <branch_name>
git rebase develop

It should be noted that rebases fundamentally change the git commit history so if you use you will need to 'force' a git push. This absolutely should not be done if others are making changes to your branch (please don't do this). If you are clear to do it then you can run:

git push origin <branch_name> --force

Worst case if you fuck it up, you have to redo your work :joy:

#### Use with caution
Although I personally think it is preferable, sometimes it's just easier to merge (if conflicts occur) so use use your best judgement on the matter and if the shit hits the fan either grab me or jason or reset your HEAD

git reset --hard ORIG_HEAD_SHA
# or if you were smart and pushed to origin before rebasing
git reset --hard origin/<branch_name>

## Out with the old
A lot of the changes that are made in our codebase's seem to hang around in PR's and branches for a long time not being merged. This is a problem in my eyes as it means that by the time these things come to be merged there could have been a significant change to the codebase which renders the change useless or requiring significant rework.

Maybe this is a problem with how we plan work and utilise sprints which needs to be changed but the way I see it, if a piece of code is completed as part of the sprint move it into the latest release and lets test and release it.

As well as this I believe we should put time limits on PR's and branches. If a PR or branch has not actively changed in say one month then it can be marked as stale, if after a week of staleness it can be deleted. [There's even a bot to do this for us.](https://probot.github.io/apps/stale/) But before we can get all fancy with robots we should cleanup the backlog of PR's and tickets we have. We should have a meeting on this.

## Links

* [Great tutorial on git by a former colleague of mine in Australia](https://github.com/charleso/git-training)
* [Git for computer scientists – more theoretical](https://eagain.net/articles/git-for-computer-scientists/)
* [Git flow](https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow)
* [Git flow cheatsheet](https://danielkummer.github.io/git-flow-cheatsheet/
)
* [Binary files in git are bad](https://medium.com/@buzaa/why-you-should-not-push-binary-files-to-git-and-what-to-do-if-you-do-29f34692cef0
)
* [Rebasing proper](https://git-scm.com/book/en/v2/Git-Branching-Rebasing)
* [Interactive rebasing](https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History)