6

I have just graduated not long ago and started to work in a startup company for almost a year. I have introduced git to my current company and they are now happy to use it for version control. I feel that my boss and the company is open to new stuff.

Recently I have heard about the idea of code review and want to give it a try in our company. However, unlike git, I have never done code review. Also, it adds extra work load and I am not sure if everybody is open to accept comment / complain during code review. Is there any suggestion on promoting code review?

closed as too broad by gnat, durron597, user40980, user22815, user53019 Aug 5 '15 at 2:00

Please edit the question to limit it to a specific problem with enough detail to identify an adequate answer. Avoid asking multiple distinct questions at once. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    You don't need a public service to do code review. – Robert Harvey Jul 29 '15 at 5:13
  • Yes, but as I am also asking for tools for code review, that comment is to make sure people won't suggest tools like github. (github do provide tools for code review, as far as I know.) – cytsunny Jul 29 '15 at 5:14
  • 1
    We don't make tool recommendations on this site. – Robert Harvey Jul 29 '15 at 5:16
  • Asking for tools is one of the top closing reasing for questions, so better remove that part from your question. – Doc Brown Jul 29 '15 at 5:34
  • Thanks for your reminder. I am new to this site. That part is deleted now. – cytsunny Jul 29 '15 at 5:37
7

Do not call it "code review" when talking to your boss - that can make the impression of a very formal technique which needs special knowledge, training and tools. Call it simply "proof reading" or "four eye principle", and suggest to apply this to all code (especially to your own code) before it goes into production. Its not unlikely when you ask another team member to review your code, others will follow your example and ask you (or some other guy) for reviewing theirs.

After you get your team accustomed to "constant proof reading", the need for better tools or a more systematic process will probably arise from itself, then it is the time to think about tools, not beforehand.

  • I call it "asynchronous pair programming". – Eric King Jul 29 '15 at 15:19
5

It's great that you're taking initiative to introduce new things to a startup - but if your founders have been through the enterprise experience sometime in their lives, then they'd be wary of any 'processes' that have a tendency to introduce bureaucracy in the company. A simple code review tool can gradually evolve into a mandatory pre-commit review process that is enforced by git hooks!

You need to be shrewd enough to not imply any such thing and steer away from any discussions that head in that way - because, well, everybody hates bureaucracy!

Here's the technique that I had once used:

  1. How do people look at each other's code right now?

    In my case, people used to email each other patches (so we kind of did have an informal review process already) - and then you could see people either walking over to each other's desk or writing poetic descriptions to locate the part of code they want to comment on

    In class SomeWeirdClassName, function fooButNotJustFoo() should return a SomeStructInADifferentHeader instead of an int!

    You can now point at such instances and say, "Hey, this is broken! We can do it in a better way!" and then go on to talk about how a code review tool allows you to add inline comments directly on a particular line of the patch.

  2. Start with a small group, maybe your own team (you can coerce, ahem, convince them over lunch) and ask them to evangelise it with you - talk about how awesome things are in code review land during an all-hands meeting.

  3. If you have an admin guy, then get him drunk on a Friday night and silently add a couple of e-mail aliases to the CC list of all reviews. On Monday, quite a few people would get the code review mails, with contextual comments, live links to the patch and what-not; by the time someone realizes what's going on and removes those aliases from the CC list. But your word is out! Now everyone is talking about "those weird mails that ended up in their inbox by mistake" - the perfect time put on your evangelist hat!

  4. If you prefer to talk directly to your boss, then make sure you highlight the fringe benefits of using the code review tool -

    a) The emails ensure that everyone knows what every other developer is working on

    b) If some developer decides to call in sick on release day, then you don't have to hack into this computer to get what he was working on - you can just download the patch from the code review tool and check it in yourself

    c) Frequently putting others' code into everyone's faces embodies a sense of the prevalent coding culture and prompts everyone to get on the same boat, as opposed to religiously following their own coding style

Lastly, since you've already introduced git (successfully) and people are happy to use it, you already have some street cred riding on you - bank on it to push this new amazing thing that's going to change everyone's life (for the better)!

  • We have a shared drive inside the company and everyone at the company can view it (our company is still small, just around the size of a team as you mentioned), and thanks to git is is also easier to see the changes of the code over time. However, it will be odd to view code that is not related to my duty, while those who is related to me should be corrected by me instead of telling the others. Not to mentioned that most of my colleagues are more experienced than me and actually I would expect I am the one who need their comments. Maybe talking to the boss directly is a better solution. – cytsunny Jul 29 '15 at 9:22
2

Be the change you wish to see in the world.
         —Ghandi (bumper stickerized)

These sorts of things tend to be much better received when introduced voluntarily, so volunteer. Put up a gerrit server or similar and start putting your own changes on it. Tell a few people you're trying to improve your code quality and ask if they wouldn't mind reviewing your code. When people ask you to informally review their code, request they put it on gerrit. Make it open for anyone to use.

At the very least, your own code quality will improve. You will find out who really cares about code quality and who is resistant. You will see how your colleagues prefer to use it, and can use that to create guidelines if it's later made mandatory. Company-wide, it might continue indefinitely as volunteer only, or if you already have a disciplined team that does informal reviews you might find out it's not worth the trouble.

What often happens is a release will have a lot of quality problems, management will start freaking out about how to prevent that in the future, and will look around for solutions. If you have this volunteer process in place that is already working well for some, it's likely to be given management support company-wide at that point. At my company, our agile process, automated testing, and modularization efforts all basically started as volunteer ideas.

0

If people don't want their work looked at by others because they fear (the repercussions of) criticism, you've a far more serious problem than a lack of code review.
Your actual problem is a toxic environment in which people are severely punished for imperfection, and have got into the habit of hiding mistakes, seeking scapegoats, glossing over problems. Most likely it's also an environment in which success isn't valued, let alone rewarded, because nothing less than perfection is concidered to be the barely acceptable minimum standard.

Unless and until that culture is broken open and people feel free to discuss problems, talk openly about where things go wrong and what should and can be done to fix them, code review is never going to be accepted and forcing it on people will only lead to them spending a lot of time trying to blame their own mistakes on each other because they know full well that the moment something is blamed on them it has instant impact on their career.

  • I think what they will be fear of is not criticism itself, but the extra workload that comes after criticism. (to edit the code as commented) The schedule is quite tight right now and we are constantly hiring. After all, this is what I imagine only. I have not yet started to promote code review, maybe they will be happy to see their improvement too. – cytsunny Jul 29 '15 at 7:58
  • @user1273587 if they produce decent code and the code guidelines/standards are well described, reasonable, and followed, there's very little rework that's not because of technical or functional problems with the code, iow bugs that would now be caught only in testing (hopefully) or production. – jwenting Jul 29 '15 at 8:50
  • Code guidelines / standards are not yet setup at this moment. I would like to make up one standard by reviewing each other's code. Will that be a huge problem then? – cytsunny Jul 29 '15 at 9:05
  • 1
    @user1273587 if you don't have a baseline to which to review, each review session will become a "I prefer it like this", "but I like this better" fight between hotheaded people, each with their own set in stone preferences. Say you like CamelCase, the other guy likes to_use_undescores, or even lpfsHungariannotation... – jwenting Jul 29 '15 at 11:05
0

First you must be sure that everybody is ready to accept his code to be reviewed (actually this can be the hardest step) and can leave his ego aside.

Then you can promote the virtues of code reviews to your colleagues (over the lunch, etc.) to make them understand what are the benefits of code reviews. I suggest you reading this article.

When you feel they are mind-ready for code reviews, you can install a tool and start reviewing your colleagues code (alone, but better with them) and assign them issues. They will receive mails and will eventually complete the issue and getting familiar with the system and start reviewing !

Not the answer you're looking for? Browse other questions tagged or ask your own question.