Expressing Debts through the code
IMHO code review is something that needs to be close to the code, that’s a very XP(Extreme Programming) way of think. Talking about Java for instance in the past I created a set of annotations so I and my team could express technical debts on the code. This was a big hit back on the time. Back on the time, you could express on the top of class or method something like:
@Refactoring So everytime someone open that class it would see a note that they need improve that code. Why create an annotation if you could create a comment with //TODO: NEVER FIX ME. People don’t fix TODO notes. :D So yes that was rebranding todo as java annotation. I think It worked out back on that time because of 3 reasons:
- My Team cared about CODE quality and that feeling was created and re-forced by the team.
- I made a simple ruby script that downloaded the source code and counts how many Annotations I had so I was running that every week and asking the team to reduce the number of Tech Debts.
- Every simple Code Review that happened as required to reduce the number o Refactoring annotations.
Back on that time, I had no Github so Code Review was done time in that TXT files, since I was doing more continuous integration approach, and yes, no branches :D Them github come with a bunch of people doing git flow(which I still think is complicated and don’t add value) and Git Hub Flow(which is simple and adds value). Today I don’t use this anymore because I work with: Java, Scala, C, Python, Bash a lot and I would need to create something like that for each language, plus the fact that my team is more senior than ever so nobody needs to say things 2.x :-) Having an sr teams means that everyone can merge PRs. IMHO JRs teams should not merge PRs as shared responsibility but concentrate in Sr member this way who knows more, in theory, does better review. Github is great for this, even for private repos, even if you don’t have to merge power you can comment which is great.
Review PRs in private repositories
The first thing that happens here is the fact that now you have a PR where you can put comments and its easy to express what you want to change. I worked with repositories that lieraraly was ultra shared and the whole company touched and I experiences worst times ever accepting prs and repos where there was more discussing and comments them commits which might sound as something code but actually was not.
Some people believe in PURITY like you cannot put a single line of “bad” code or “style” in your code base. I’m not like that. I believe in Waves and Progress Principle. You don’t need to pay all tech debts at once, you don’t need to fix all the things in the same PR. It’s good to make progress. Having a PR sitting on the repo for days and they receive a list of 50 changes is the wrong thing that could happen. So there are lots of waste doing things that way and I do very bad for the team morale.
Kanban W.I.P Limits applied to PRs
Took me some time to realize but( I don’t know how I forget this :-)), Reducing WIP increases the Throughput, a very Lean Kanban principle. How we do we limit WIP in PRs? Limiting the number of PRs? Well, that’s one way but I discover that limiting the number of FILES was way more effective. Currently in my team and the repos we “own” LIMIT PRs to 10 files MAXIMUM. There are exceptions like you need rename the project this will make you go beyond 10 files, however, it is a feature or bug fix we limit in 10 files. This also creates several benefits like:
- We end up spending less time in code review
- Code Review is way more easy and easier to understand
- People end up finishing more tasks(Progress Principle) — this creates a positive feeling
- More PRs means more tasks so you need split your work — also removing waste
- Merge time is very fast and nothing sits on PR queue(Kanban) and (Github)
- Feedback travel fast and speed up delivery and fix time
One thing my team does a lot has discussions on Slack rather them in Github because is faster and don’t pollute much the tread. Sometimes we need to have 10–15 minutes chat to decide whats best todo. I guess this is a matter of preference but Slack is faster to time them via github comments. Sometimes we are in different timezones with a 4–6h difference so we end up using Github more.
Clean Code / Space VS Tabs and Other ways to waste time
There are some properties that are no doubt important. I try to separate this props from “style” as much as I can. I don’t see value creating a huge style convention on wike or IDE file that is never updated and people stop maintaining over time. IMHO What matters:
- Proper Error Handling / Fallbacks
- Code Reliability
- Avoid unnecessary Classes / Methods
- Typos that you might miss it
- Basic Clean Code
Why do I say “Basic Clean Code” because I mean not have a method of 100 lines of code or having 20 if statements in the code Basic Clean Code also stands for some “reasonable” naming and avoiding very long names that mean nothing This might get subjective and people think different there were might a trap happens and you end up having pointless philofhial discussions. So there is some level of “Bad Clean” code which I try to avoid. Spaces vs Tabs is a pure waste of time discussion for me and really does not matter. IMHO the only thing that matters is people use same chars for tab or spaces, that’s the only thing. IMHO the big thing is to have as many methods as possible because with methods you can explain what your code does, so you explain by telling why you are doing something.
Code Review need to work with Tests(Unit and Integration) — Currently, on the last 4 years I’m not working just with microservices but with DevOps and NoSQL Engineering so Integration Tests matter a lot for me(On microservices often unit tests matter more). Tests are nothing without Clear Design and Proper Architecture so this is just one element of many elements of an a successful solution.
Originally published at diego-pacheco.blogspot.com.