While working tech support the other day, I tracked down, with the help of a couple of other developers, a curious issue reported by a customer. They had Code Reviewer, the lightweight version of our peer code review tool, installed and working, but could only login to the web UI from the server machine itself -- and even then only by using http://localhost:8080. Attempts to use http://host_name:8080 or http://host_name.domain.name:8080 failed. They could reach the server and get the login form, but upon submitting the form, users were not being logged in.
As it turns out this is a wellknown "feature" of Internet Explorer, and possibly Safari. It does not store cookies when there is an underscore in the hostname or domain name. Since this particular customer had an underscore in the server's host name, Code Reviewer's session cookies were not being stored or sent with requests. Thus the user was perpetually logged out.
I suggested that the customer take one of three steps to resolve the issue:
Use Firefox because it handles hostnames with underscores.
Rename the server machine (or at least get a DNS alias for it). This way users could access the server with the alias, which would not have underscores, and cookies would work fine.
Use .hosts files to simulate a DNS alias. Since this was a small group (they had taken advantage of our 5 for 5 deal so I knew they were small), so updating all the relevant .hosts files was not too much of a burden.
Ultimately, this particular customer chose the .hosts file route and they are now up and running. If you run into this issue, with Code Reviewer, Code Collaborator, or any other web application, you can choose any of those workarounds and it will probably resolve your issue.
Now for the rant. What was the Internet Explorer team thinking? It seems they made it as difficult as possible for users to understand what's happening. Instead of secretly eating the cookies, why not let the user know what's happening? Give them a choice in the matter or at least tell them they don't have a choice. Or, better yet, do like Firefox and try to do the right thing for your users, even if it means accepting cookies that are in violation of various RFCs.
For example, IE could simply not navigate to sites with underscores in the hostname. If violating RFC 1123 is grounds for having your cookies ignored, maybe it should be grounds for being entirely inaccessible. At least this behavior would be consistent and, provided there was sensible error messaging, it would give unwitting users with control of their hostnames a chance to fix the problem.
Alternatively, make it a warning and give the user the opportunity to decide whether or not to navigate to the site. If the user does navigate to the site, it should work like the user would expect any other site to work. Respect the user's security preferences with regard to cookies. No special behavior.
If you then assume that most people will not read the warning message and will instinctively click the "Yes, please do what I asked you to do" button, then you remove that button and you are right back to where Firefox is. If you can resolve the name and access the server, it gets no special treatment. And that is exactly how it should be.
If you blinked, you might have missed it: the chance to get 5 licenses of Code Reviewer 5.0 for only $5!
For the first five days of June only, we ran our own economic stimulus package, offering five seats of our Code Reviewer online code review tool, complete with full tech support, for only $5. Then we donated the $5 to Wikipedia, one of our favorite websites and a most worthy cause. Their goal is to provide every person on the planet with “free access to the sum of all human knowledge” – pretty cool!
Code Reviewer is a lightweight version of our powerful Code Collaborator software. It’s ideal for small teams that need an economical yet fast and convenient way to collaboratively review each other’s code. We know it’s hard to secure budget just now, so we wanted to do our part to help out small teams and independent folks gain access to this time-saving, hassle-free tool to improve their software quality. And of course, we’re thrilled to do our part to help spread knowledge worldwide on Wikipedia.
This time, this special opportunity only went out to our newsletter subscribers. Thanks to our avid readers, we donated more than $2000 to Wikipedia in June!
Maybe we’ll do it again. Stay tuned, or drop a note to "sales 'at' smartbear.com" if you want us to notify you directly. Or just subscribe to our newsletter!
I'm the default linux geek around here, and since I recently did some pre-publication review of a friend's git book, I thought it was time to switch my at-work work flow over to git. This step was taken with some trepidation due to the immaturity of various tools in the git ecosystem. For example our own product, a code review tool, has support for git, but not as much support as I would like.
Since our main repository is in Subversion, I had to use git-svn to interact with it (this intro was quite handy). This post describes the work flow I came up with to make use of git-svn with Code Collaborator.
First off, I had to do the appropriate 'git svn init' on my master branch to import the existing Subversion repository. Since I plan on using this long-term I went ahead and pulled in all of history. This took awhile, but since it is a one-time operation, that's not a problem.
Once that was done, I was ready to work, so I branched off a topic branch (git checkout -b topic) to keep from sullying the master branch with work that might not be committed for some time.
I figured out what needed fixing in the code and then fixed it. I went ahead and committed to git (git commit -am "topic description") as I was going along to make sure I didn't lose any work and could backtrack on some of my more experimental coding choices if need be. Once it was working and tested, it was time for a code review.
'ccollab addgitdiffs new master' created a new review on our Code Collaborator server where my co-workers sternly criticized my Javadoc typos. After fixing the typos and committing again, I submitted the new versions of the files to the code review (ccollab addgitdiffs #### topic^).
By now my colleagues had gone on to other things, and I was able to do the same even as I waited on them, simply by making a new topic branch from the master (git checkout master ; git checkout -b topic2) where I could do work on a new code change. When I got to a stopping point in the new work, I went and checked the status of the code review and sure enough, this time my colleagues approved, giving me permission to commit those changes to the trunk.
First, though, I had to update to the latest version of the trunk by switching over to the master branch (git checkout master) and updating it to the latest from the Subversion repository (git svn fetch). Then I could switch back to my topic branch (git checkout topic) and rebase it onto the updated master (git rebase master).
Now I was ready to coalesce my code into one commit (git rebase -i and judicious s/pick/squash/) and then merge it onto the master branch ( git checkout master ; git merge topic ). Once merged, I pushed it to the Subversion server ( git svn dcommit ) so that the rest of the team (and our continuous integration server) could pick it up.
Simple enough, right? I'm sure there are improvements to be made - I tend to stick to simple git commands that I know will work instead of the more complex versions that could probably combine several of the above steps. Further, there are some things that tooling could help with (including Code Collaborator), but overall it's been pleasant to be able to switch what I'm working on more quickly than before (when dealing with multiple svn trees) and to track intermediate work on a single topic without worrying about 'dirtying' the main tree. I look forward to trying out the Eclipse git plugin and hopefully adding better git support to Code Collaborator.
After a successful beta test period, Code Collaborator v5.0 has been released! This is one of the biggest releases yet of our award-winningpeer code review tool, and includes features such as:
Support for reviewing additional types of documents: PDFs, images (JPGs, PNGs, GIFs), and URLs
Additional user control over review event notifications
A better diff viewer, including the ability to compare any two versions of a file
Post-commit review creation directly in the browser - no client-side application required
There will be a webinar on June 30, 2009 to demonstrate the new features and answer questions - click here to register. (For our complete webinar schedule, or to see a replay of a past webinar, click here.)
If
you are already using Code Collaborator you can upgrade just by
downloading the new installers and running them. Of course you'll want
to backup everything first. Or, make a copy of your existing installation and try v5.0 on a test machine. More details on these options is available here.
If you are not currently using Code Collaborator, you can get a 30-day free trial.
Either way, if you have feedback, please let us know. Leave a comment on this blog, send us an email (info at smartbear dot com), or add an entry to our public feedback forum.
The code "walk-through" is the most common kind of code review. Whether it's really informal with a reviewer hovering over the author's shoulder or a walk-through meeting with several people and a projector, a "walk-through" is when the author runs through his changes and gets feedback from others.
I don't like walk-throughs. Here's why.
1. No Context inside the Code
The next developer who has to understand or maintain the code won't have the benefit of the author walking him through it. Too often reviewers accept what is told to them because it makes sense in the conversation, but the important information doesn't make it into the code.
2. Shallow
To understand complex code takes thought. You have to look up related code in other files. You have to run through possibilities of error-handling and thread synchronization. When you're being walked through the code there's no time for finding the subtle problems -- exactly the ones that are hardest to find later.
Consider a bug fix in a simple method. You see the fix in the walk-through and it's fine. But what other code depends on this? Does that code depend on the old behavior? These are questions you can easily answer when you're looking at the code alone with your IDE, not in a meeting.
3. Driven
Too often the author drives the meeting, explaining why she did this and that. Sometimes it's questioned, but often the participants are playing a matching game -- "Yup, the code matches what she said she did" -- rather than thinking deeply about the code.
4. Time
Meetings take more time than individual reviews. It's also easy to get sidetracked on tangential discussions, although a good moderator can mitigate this problem. A harder problem is scheduling meetings for people who are separated by many timezones.
Some concessions...
Walk-throughs can be a better forum for teaching new developers and generally sharing and learning from each other. Instructive conversations happen in person that don't happen in lone reviews.
Sometimes it's nice to have multiple people in the room, especially when there's a domain expert plus a language expert plus a product guru.
Therefore, in the end I suggest examining the code alone (but you still have to communicate!) as the default mode, but at the same time don't prevent people from doing walk-throughs when they find it useful.
There's a reason why most enterprise applications are written in Java, not Ruby, and it's not because Java programmers are stupid.
The main argument I'm hearing recently is that "more lines of code for the same job is automatically bad." This is clear and sensible on the surface. More lines means more chance for a bug. More lines means more to comprehend when you're maintaining code. If projects get too big, no one can understand it completely.
But...
This glosses over the fact that many applications have no choice but to be millions of lines of code. I know, 37signals wrote an application in 600 lines of code and they are millionaires. But AutoCAD is going to be big, Quickbooks is going to be big, FireFox is going to be big, and yes, even a code review tool like Code Collaborator is going to be big.
So let's pretend that FireFox was rewritten in Ruby, so instead of being 10 million lines of code it's only 5 million. I don't think that sounds any easier to maintain -- 5 million lines of code is still more than anyone can fathom.
So my question is: Which language makes it easier to work on large code bases?
In Java I can refactor with confidence. Move code around, change signatures, add a function to an interface, and I know nothing is broken. Python can't.
In Java I can say "Show me all the places where this code is called." When changing the behavior of a method, this is critical, especially in a large code base where you have no way of tracking all those callers. Ruby can't show you that.
In Java if another developer checks in a change that breaks types and signatures on what I'm currently working on, I see it immediately and I can fix it now. Big projects mean lots of programmers which means things like that happen all the time. In Python I won't find out until run-time, and even then I might not find out until it's at a customer site, doing something I didn't test for.
The second reason I don't like the "more lines of code is bad" argument is that not all lines of code are created equal.
It's true that in Java you tend to have lots of filler code. Bean-style fields are a simple example -- instead of just having a field that other code can access you typically make a "getFoo()/setFoo()" combination, and then you add JavaDoc to that, and soon you have 20 lines of code instead of 5. Another example is in patterns like Adapter and Composite where you end up doing a lot of pass-through code.
The typical counter-argument is "The compiler can write that for you." The typical response is "Yes but I still have to read that and make sure it's not doing anything funny if I'm a client of that code."
True, but still those lines of code are unlikely to have bugs, particularly if you have unit tests. More work, yes, more lines of code, yes, but more bugs as a result? No.
The fact is, most of that kind of code is written once and never changed, and that's fine. Does that really make the system more difficult to understand?
Plus, in this day of unit testing, if you're doing that well it means you can leave even more code unexamined. Just seeing how it works (the unit tests and documentation) is enough.
Yes, you can unit-test your Python and Ruby code as well, but that doesn't stop another developer from injecting a method into your class at runtime that breaks everything.
In the end it comes down to which trade-offs you like best or which match the goals of the project. In theory anyway.
But to me, I need a good reason to abandon a JVM that's faster than a C compiler, IDEs like Eclipse, tools for correctness and profiling and debugging and analysis, the vast array of quality libraries, and millions of people who are already familiar with the environment.
"Fewer lines of code" and "I don't need types" isn't enough.
Not everyone uses the same approach for doing code review. Some folks are very informal, others have a rigidly defined process, and there is a wide range in between.
This presents a challenge for us, since we sell a code review tool. We want to enable folks to use whatever process (or lack thereof) they want and have their approach well supported in our tool. The tool should adapt to you, not the other way around.
The end result: many dials and switches you can turn to modify the behavior of Code Collaborator.
For example, the out-of-the-box behavior is that all participants in a review must read all comments. This can lead to what Christopher Owen refers to as "a game of whack-a-mole, but not as satisfying." You read all the comments you haven't seen yet, click Finished, and then later someone else adds another comment so you get another notification from Code Collaborator.
There is a way to stop that in v4.0 (leave a comment if you want to know more), but in v5.0 we've improved all of this dramatically. In v5.0 there is a setting for each role called: "Required to Read All Comments." The default is "yes," which means the default behavior is still the same as it was in v4.0: all participants must read all comments.
Changing the setting from "yes" to "no" is much more useful in v5.0, however, because of an additional enhancement to the user interface. The Finished button used to be your only option, but in v5.0 there is also a Waiting option, which is handy if you have a question or concern and therefore want to prevent the review's status from being set to "Completed."
After you have examined everything you want to see in a review, you now have a choice: you can Waituntil a specific event occurs or you can indicate you are Finishedunless a specific event occurs. You won't get any notifications from Code Collaborator about that review until the selected event occurs.
If you select Finished and the selected event does not happen and the remaining participants also choose Finished, then you will not get any more notifications about that review and the review's status automatically (and happily) changes to Completed.
The list of events (from least to most specific):
Any activity - pretty much means what it says - if anything happens (a comment from some other participant, a new file upload, whatever).
Author activity - anything done by one of the participants who is an Author for the review.
File activity - file additions, uploads, etc.
Someone pokes me - v5.0 has a new user interface control that sends a notification to a review participant, so someone has to specifically "poke" you in order for this event to occur.
As a result, in v5.0 each participant gets much more control over their level of participation in each review. And that's especially true if their role has "Required to Read All Comments" set to "no."
So the answer is "No, you don't have to read every comment," assuming Code Collaborator is configured for that sort of process.
It's always fun when someone else makes my arguments for me, especially when that someone is Jeff Atwood, writer of the super-popular-and-deservedly-so Coding Horror blog and principal of Stackoverflow. (Yes, I'm an off-and-on "Crackoverflow" addict, as you can see).
Jeff has praised code review in the past... and I quote (from here), "I believe that peer code reviews are the single biggest thing you can do to improve your code."
Yeah, well, that about sums it up!
Lately he's been talking about code review in the context of Stackoverflow, like how there was an encryption problem that would have been caught by another pair of eyes.
But my favorite is from four days ago where he repeats one of my favorite aspects of code review, and which I talk about in my book (page 102) -- what I called the "Ego Effect."
Every software developer has the desire to look smart and competent in the eyes of her peers. Well, OK, perhaps not every developer, but every one who cares at all about her work.
When you know you're going to get a code review, the way you write code changes instantly. You're going to be extra careful to document properly, do input validation, handle corner cases, write good tests, and think about scalability and error-handling.
Why? Because the last thing anyone wants is to be torn down during a code review! You want an airtight review -- no possible way to improve. See how awesome you are?
Say "code review" and instantly everyone in the group writes better code. Is there anything else with so strong and immediate an effect?
This works no matter what kind of code review you do! It's not about tools or process or meetings or metrics or anything -- just peer pressure.
Jeff calls the Ego Effect a "paradox in motivating developers" -- that to motivate them you have to tell them they "suck."
But that's not it. It's not telling someone "you suck" that's motivational. The motivation is that someone else might think you suck. And there's nothing paradoxical or unhealthy about wanting to look awesome.
We've gotten quite a bit of feedback about which features we should add to our code review tool. One of the messages we heard quite clearly was: "Let me compare image files!"
So one of the key features in v5.0 of Code Collaborator is the ability to compare GIF, JPG, and PNG files. It works more or less the same way as comparing text files, with some important differences.
A big difference is that line numbers are of no use for tracking comments and defects in an image file. So Brandon came up with a push-pin user interface that records coordinates instead.
Another key difference is that Code Collaborator does not highlight the differences between two versions of an image file, like it does for text files. A more useful approach, it turns out, is to provide user interface controls that allow you to easily pick out the differences. So in addition to a typical side-by-side view of two versions of a file, the diff viewer also supports an over-under view.
When you use the over-under view with images, the image that was displayed on the right is on top. And initially it is partially transparent so that you can "see through" it to the image below it, which is the image that was on the left in the side-by-side view.
The really cool part is that you can alter the transparency with a slider control. The best way to understand all this is to watch it in action. Check out the video and then if you want to try it for yourself, download Code Collaborator v5.0.
Meet Paul Jimenez. Also known as PJ, he's the Senior QA Manager at Smart Bear and quite the popular bear. If you've ever called our Austin office for a Code Collaborator question or problem, chances are you've heard his friendly voice. So, get to know who's on the other side of the line.
Describe a typical day for you at the office.
A typical day at the office involves one or more of: writing bugs...er, features, fixing bugs, figuring out how to better test our software, wrangling with our Continuous Integration server, or putting on my other hat of System Administrator and wrangling with the bug tracker, the Xen install, various hardware, or the phone system.
Where were you and what did you do before Smart Bear?
Just before coming to Smart Bear I was a Senior System Administrator for a local hedge fund, building out and managing their network and compute cluster. Before that... well, it's basically been linux and system and network administration with smatterings of perl, java, and python since around the time of the first ISPs.
How did you come to Smart Bear?
I knew one of the other bears, Roy, via a local juggling club, so when I went looking and he said they were hiring, I jumped at the chance.
What is your favorite thing about Smart Bear?
The other bears - they're all smart and engaged in what we're doing. It's great to be part of a team that cares about the product we're creating and the customers who will be using it.
What are some of your favorite websites?
I spend more time than I probably should with a browser pointed at Google Reader, scanning a couple of hundred newsfeeds, mostly by prominent net personalities (Lawrence Lessig, Seth Godin, Tim Bray, Bre Pettis, Jamie Zawinski, Wil Wheaton, Guido vonRossum, etc) or on topics I'm interested in (Linux Weekly News, Groklaw, The Java Posse, etc).
What are your favorite activities? Hobbies?
My family gets most of my time these days, but I still find some time to write software for fun, contribute to open source projects, read whatever's handy, and juggle.
Favorite music bands?
I've got wide musical tastes, from 80s rock to country to trance to irish/celtic and reggae, but few favorites. They Might Be Giants is always fun, but I go for trance or house music when I'm working since anything with vocals distracts me.
Favorite spots in Austin?
The various dog parks with our dogs, or the playground with my son, or (much more rarely) Opal Divine's (either Marina or downtown) with a single malt neat, tall water back.
