Our group is starting to use Review Board more and more to conduct our code reviews. But using it with git-svn, or with anything you’ve already checked into svn can be a real hassle. You don’t want to have uncommitted code hanging around in your computer for extended periods of time for a number of reasons, so this rendered it difficult to use for code projects that take more than a week.
Since I like to eat, and hence get paid so that I can afford food, I have to work on things that take longer than a week. But luckily, git can help get a diff for review board based only on a string in the commit comments.
This came about when one of my co-workers posted a bash scripts someone had written that would take a git diff and make it into a svn-like diff. And that worked great. My first attempt was to modify it to do multiple files at once, but it ended up making a diff for each revision for each file (if you need that functionality, you can get that here).
So I pulled out python and worked on a new script, which is at the bottom of this post, or you can get it here.
Once you download the file, you run it like this from your git repository:
git-create-review.py (some grep string) > diff.output
The script will take the argument as a string to search for in the commit comments. This is thanks to the git log –grep=foo command available in git. It outputs directly to the terminal, which you can see I’m redirecting into diff.output. Once you have the file, you can upload to Review Board as you normally would.
I did it by using a search string because we use JIRA to track our work in svn. We have pre-commit hooks to ensure that we put a JIRA number on each commit. This helps out git as it has something to search through to get the change list.
Known issues:
First: It will get all the changes between the first and last revision that had the search string.
Example:
I change File.java under JIRA-123 (now at r1). Another change to File.java under JIRA-321 (now at r2). I change the file again under JIRA-123 (now r3).
This script will take all changes between 1 and 3, even if I don’t want the JIRA-321 changes to show up.
Second: it doesn’t have any context around removed/moved files. It still should pick up a renamed file as just a completely new file, but it doesn’t have any context of renaming. But I believe that’s the case with svn anyway.
Third: I am by no means a good python programmer, and might not know all the conventions that people normally follow. The code below may cause eye hemorrhaging, face hemorrhaging…pretty much any of your organs could be hemorrhaged by looking at the code. CONSIDER THIS YOUR ONLY WARNING.
But I did some testing with newly created files, and removed files, and binary files, but I’m sure I missed some crazy edge cases. Let me know if you hit any snags.
The script follows:
If you go to the bottom of the page for git-svn, under the “Basic Examples” section, you’ll see in there a procedure to clone a git-svn repo that someone has painstakingly already cloned from svn. That’s great, because it will save you hours and hours of mind numbing time that would normally take someone to clone an svn repo.
That person may be tremendously happy…that is, until, you try to “git svn fetch” to get new revisions and branched and what have you, and you get an error message like this:
Last fetched revision of (some branch or tag) was r3421, but we are about to fetch: r3421!
In an attempt to fix it, you may find this blog post, saying to remove the .ref files from your svn directory, which also proves fruitless.
The fix: modify the .git/svn/.metadata file so that the branches-maxRev and tags-maxRev equals the latest commit revision in svn. So it should look like this after you are done:
; This file is used internally by git-svn ; You should not have to edit it [svn-remote "svn"] reposRoot = (svn server name) uuid = c08781d2-f03e-0410-8c7c-e884ea3e41f3 branches-maxRev = 17224 tags-maxRev = 17224
Where 17224 was our latest revision.
I got the idea from git-svn’s webpage, where it states:
If the subset of branches or tags is changed after fetching, then .git/svn/.metadata must be manually edited to remove (or reset) branches-maxRev and/or tags-maxRev as appropriate.
I must caveat this with the fact that I have no idea why this works. Git svn is still black magic to me, and making this work is akin to me throwing muskrat bones onto a flaming pyre, and drawing conclusions from the charred remains. Maybe someone with more git knowledge could drop it off here?
So, you have a materialized view in your DB. That’s great. Give yourself a cookie.
You also have created a set of tests in your DAO layer, using the AbstractTransactionalSpringContextTests. Good for you, again.
But when you try to mix these two together, you may see tests failing and can’t immediately see why. This can happen if you try to get data from the materialized view after attempting to save data to a table that the view accesses.
Well, it’s very simple, and probably pretty obvious. It wasn’t immediately obvious to me why it was failing, and I didn’t really see anything online about it, so hopefully this will stop your search quickly.
First thing to realize is that running your DAO tests through the AbstractTransactionalSpringContextTests is that every test is run in a transaction. That’s great most of the time. Each test will run in a little isolated environment and the changes to the database will be rolled back at the end. You don’t have all kinds of test data strewn about afterwards as if it were the island in Cast Away.
The second thing to realize is that a materialized view will only be updated on commit. There’s a lot more to it than that, but I am not a DBA and that’s all you need to know in general.
So, with those two things in mind, you can see how it might be a problem if you try to write tests against code that uses the materialized view. All is not lost however. If you put an endTransaction() call somewhere in your test, all of the DB calls you’ve made up to that point will be committed. Also, any other calls made after that point will also be committed to the database. Since that happens, you will be on your own to clean up after the test as you see fit. Also, placing an endTransaction will only affect the test you place it in. Any other test will still continue to run and rollback in the normal fashion.
What we ended up doing was making the tests that need to commit to the database small as possible. We then wrapped the endTransaction() function in a function called setTestToCommit() (my co-worker James’ suggestion), so that it makes more sense to read it, and placed it at the beginning of the test.
There’s lots more information about how it works here.
Conky Hudson plugin
I just finished updating the conky hudson plugin. You can get more information on main page here. Hopefully, you like it. If not, then…what do you want from me?
