Add post 'Prevent accidental debugging commits'

This commit is contained in:
Ariejan de Vroom 2014-10-15 10:26:19 +02:00
parent 0ad97ed9e7
commit ac9c2bd181

View File

@ -0,0 +1,69 @@
---
title: "Rails: Prevent Accidental Debugging Commits"
created_at: 2014-10-15 09:59:27 +0200
kind: article
tags:
- rails
- git
summary: |
Figuring out complex app behaviour in development often requires you to pull some tricks. It would be very awkward if those changes found their way into production unintentionally.
---
Your Rails app has grown over time, multiple developers have worked on the code and you are about to make a small change in the code.
Unfortunately this change applies to code that allows you to view a financial overview of last years data. Its legally not allowed to expose the current years data, but the change you need to make applies only to 2014. We should shift time to 2015. Problem?
There are several things you could do.
1. Change your system date to 2015. This has the downside that many apps get confused, including, most likely, your backup tools.
2. Use a gem like TimeCop to fake time. This works great for tests, but it feels a bit like cheating in development.
3. Change `@financial_year = 1.year.ago` to `@financial_year = 2014`.
_Side note: This feature gets tested by both unit and integration tests using TimeCop. The problem here is viewing and testing the change in my browser, prototyping the change and showing it to the customer._
Option 3 seems like the easiest. In the scope of the financial overview, lets pretend were always viewing data for 2014.
The real issue here is that you forget you made this change and commit it. Although there are several process in place to detect this: peer-review, QA-testing and custom acceptance. There still is a chance this change makes it to production. _Whoops!_
Because this is Rails there are lots of awesome goodies around, like `rake notes`. It finds comments that start with `OPTIMIZE`, `FIXME` or `TODO` and lists them for you. Its not unlike what many IDEs do you as well.
Instead of this change:
```diff
- @financial_year = 1.year.ago
+ @financial_year = 2014
```
I could make a change like this:
```diff
- @financial_year = 1.year.ago
+ # TODO: Change this back to 1.year.ago!!!
+ @financial_year = 2014
```
Then, before committing, I should run `rake notes` or `rake notes:todo` and check if theres anything that needs to be reverted back. But Im a programmer, dammit. I can automate this! I _should_ automate this!
Using a combination of a git pre-commit hook and a custom annotation using Rails `SourceAnnotationExtractor`, the core of `rake notes`, it should be possible to simply reject a commit. Rejecting a commit for pending `TODO`s doesnt seem useful. Instead, lets use a custom annotation: `NOCOMMIT`. The code change is like this:
```diff
- @financial_year = 1.year.ago
+ # NOCOMMIT: Change this back to 1.year.ago!!!
+ @financial_year = 2014
```
And with the following in `.git/hooks/pre-commit`:
```sh
#!/bin/sh
nocommit_notes=$(ANNOTATION=NOCOMMIT rake notes:custom)
if [[ -n "$nocommit_notes" ]]; then
echo "Error: You have NOCOMMIT notes in your code."
echo
echo "$nocommit_notes"
exit 1
fi
```
Using the NOCOMMIT tag will prevent you from accidentally committing changes you do not want committed. This is an extension to other practices, like interactively staging your changes and reviewing what goes into a commit, code reviews and acceptance testing. Its yet another safe guard agains my own stupidity.