For a team not used to code reviews, they might seem more trouble than they're worth at first. Most likely they will be more trouble than they're worth for the first few months. Keep doing them and eventually your smart developers will figure "if we have to do this anyway, we may as well find something useful to say" :)
A few things you can do to make it smoother:
- Manage expectations. Initially it may be as simple as "we just want to have a second pair of eyes on every change" or "be aware what other team members are up to" - i.e. communication first, improving code second.
- Set up your tooling to make the process smooth. If somebody wants to just get it over with - it should be easier for them to use your official review process then to use some side channel. A vicious alternative is to make using side channels harder ;)
- Leverage automation. Run tests, linters, static checkers, etc. on your PRs so that developers get something useful even if no human leaves interesting comments.
- If some team members already have experience with code reviews - adjust their workload so that they can do more reviews. They are effectively training others by example.
- Make sure that code changes under review are reasonably sized. Encourage submitting changes for review early and often. "Here is the feature I worked on for 3 months, and it's due on Friday, please review" won't make anybody happy.
- Make it less intimidating. Code reviews are not just for finding bugs and flaws, encourage reviewers to say positive things as well.
This is quite good advice, I feel the “we have to do this anyway” line is more like… “so we might as well make it easy for ourselves”… eg write code that works, you self tested it through tests and manual if needed, so the reviewer doesn’t have to get bogged down in actually running it (start by adding screenshots for this but graduate to not needing them). Keep PRs as small as possible, aka multiple PRs streaming after each other for a single feature card, get the PRs in as soon as valuable and don’t block for nitpics but the shared expectation you start to agree on things that are better and they happen with the next changes.
The general mantra being that “if it works then it shouldn’t be blocked” and developer can choose to improve the maintainability there and then or delay it to next or later PRs at their discretion. After all you trust each other.
> Leverage automation. Run [..] linters, static checkers [..]
These don't make the process smooth unless you set them up to simply give a warning rather than block the build/merge. And with that they'll likely get ignored anyway.
I think linters/etc should be avoided until you already have buy-in from the team.
It depends. If your codebase is already free of lint warnings - adding a blocking check to prevent new ones is no big deal. But if your blocking check means that everyone has to drop everything and spend a week fixing code - of course this won't be smooth.
PS. Also, it’s a good idea to have manual override for whatever autoblocks you set up. Most linters already come with this feature.
IME even better than linters is something like `go fmt` (I work mainly in Python and use https://github.com/psf/black). Rather than forcing you to manually jimmy the code to satisfy the robot, just let the robot make it look the way it wants.
> A vicious alternative is to make using side channels harder
If people are looking to side channels, that is a signal. Ignoring that, ignoring that people are trying to find ways to do their jobs effectively - I think seems to be missing the point.
> If some team members already have experience with code reviews - adjust their workload so that they can do more reviews. They are effectively training others by example.
This can backfire. Suddenly it's just a few (potentially just one or two) team members doing all of the code reviews. They feel pressure to not be the blocking part of shipping, their reviews are then done hastily. They LGTM rubber stamp stuff from the other seniors and probably punt on reviewing the other reviews until the next day. They struggle to get their own work done, 2 hours a day code reviewing is a huge impact (if reviewing work from 2 other developers, who have been jamming out code for 6, 8, 10 hours in one day - it will take 2 hours to review that the next day).
> - Make sure that code changes under review are reasonably sized. Encourage submitting changes for review early and often. "Here is the feature I worked on for 3 months, and it's due on Friday, please review" won't make anybody happy.
On the other side of the coin, "Hey, review this really quickly so I can give you a series of 4 more changes in the next two hours, before finally sending the update that does the thing."
I believe Norton Commander was the original product (for MS DOS), Midnight Commander is its open-source cross-platform clone. FAR Manager is another notable TUI clone of NC.
Windows Commander/Total Commander are also NC clones, but implemented as GUI instead of TUI.
NC was one of the most convenient UIs for managing files, it’s a pity none of the major operating systems adopted this style for their default file managers.
Volkov Commander should get an honorable mention here, it is a tiny reimplementation of the NC interface. On the FAR Manager level of complexity DOS Navigator is also notable.
The first time this happens the programmer will spend hours looking for the first few places that need to be changed. Then he will spend days debugging, and hopefully will find the last few places.
Then, even though he is now informed about the connections, he will NOT refactor this code because he (a) has no time to refactor as he already spent a week on what seemed like a one-line fix, and (b) is now terrified to touch this code any more than absolutely necessary.
> Fire and Motion, for small companies like mine, means two things. You have to have time on your side, and you have to move forward every day. Sooner or later you will win.
It's interesting to reflect on this 20 years later. Can we say that FogCreek won?..
Probably just there for legal reasons. They do mention their compensation on the site however.
> If successful, we transfer your money to you immediately. If the airline refunds your ticket within 7 days, our service is free of charge. After that, you only pay if we are successful (commission 14-28% plus VAT). For compensation claims, the commission is usually 20-30 % (plus VAT).
If it's under 7 days it's probably because the airline is just less sketchy and it's super easy for them to get the money for you. The ones that take longer are probably the ones that are a bigger pain in the butt.
Had a similar story with British Airways. My flight was cancelled last moment and I was rebooked for the next day - changing my direct flight to an inconvenient connection and arriving to the final destination over 24hrs late.
When I filled out compensation form they first lied to me about who operated the flight: it's not us, it's our codeshare! Once we sorted it out they lied about the distance: your disrupted journey was less than 1,500km, so here is less then half of your compensation (for a flight from London to San Francisco). I eventually got my full compensation, but it took several months of back and forth.
Airlines will absolutely lie to you, in writing, about the things that are trivial to verify. I guess there's no downside to them - in the best case some people may settle for no or smaller compensation, in the worst case they pay what they were supposed to pay anyway.
AF also tried to pull the codeshare thing with me once. Didn't want to go around in circles with customer service and found it easier to initiate litigation. AF then hired a big law firm to defend the tiny case.
A week before a scheduled court hearing their lawyer calls me to negotiate. I tell her there is nothing to negotiate about, and she agrees. We chat for an hour anyway which was surely billed to her client AF. I receive full payment two days later.
Interesting, I had a comp case with BA but it was very easy. Likely because they were clearly at fault (mechanical failure) but all I had to do was fill in one form and I had my money 3 months later.
I think the experience is circumstantial and definitely depends on how 'obviously' in fault the airlines is. All in all I came out with a profit of £200 but can't say 24 hours at an airport is worth it honestly.
For a team not used to code reviews, they might seem more trouble than they're worth at first. Most likely they will be more trouble than they're worth for the first few months. Keep doing them and eventually your smart developers will figure "if we have to do this anyway, we may as well find something useful to say" :)
A few things you can do to make it smoother:
- Manage expectations. Initially it may be as simple as "we just want to have a second pair of eyes on every change" or "be aware what other team members are up to" - i.e. communication first, improving code second.
- Set up your tooling to make the process smooth. If somebody wants to just get it over with - it should be easier for them to use your official review process then to use some side channel. A vicious alternative is to make using side channels harder ;)
- Leverage automation. Run tests, linters, static checkers, etc. on your PRs so that developers get something useful even if no human leaves interesting comments.
- If some team members already have experience with code reviews - adjust their workload so that they can do more reviews. They are effectively training others by example.
- Make sure that code changes under review are reasonably sized. Encourage submitting changes for review early and often. "Here is the feature I worked on for 3 months, and it's due on Friday, please review" won't make anybody happy.
- Make it less intimidating. Code reviews are not just for finding bugs and flaws, encourage reviewers to say positive things as well.