Code review etiquette

With the process of submitting for code reviews and also doing code reviews, I’ve jotted some  etiquette or “time saving” tips if you will, on the process. This is going to address more of the interpersonal skill part of it, than the technical aspect. I am avoiding addressing the technical aspect of code-review since well, in my opinion there are a zillion ways to address a technical problem, and each coder assumes she is taking the best approach. It sucks that I lost my ace reviewer, it was fun knocking that extra line of code and seeing a beautiful piece of code checked in.

As a reviewer: 

1. Please be prompt with the code-review. There is someone waiting at the other end to submit a change.

2. If a “modification” goes beyond an iteration or two its either time to draw up that chair and share the cube  or pick up the phone and discuss.

3. When providing feedback, please be specific, its best to provide examples in terms of code. That way you are making it clear to what you as a reviewer expects.

4. Be kind, as a reviewer you may come across a lot of levels of coding capabilities. Some reviews may just require simple changes to make the code more readable, and in others it may need major  code refactoring.

5. If you are just starting to code review someone’s code, then don’t pick on ever thing that you think ought to be changed. Start in steps, once the submitter improves on the code, increase the level of expectation.

6. Please don’t file bugs for every enhancement, lay it out as a suggestion that the submitter can think about, if the change is mandatory, then please file a bug.

7. Please encourage the submitter for good work done. This keeps the tone positive.

8. When discussing ‘problem solving’ from [2], start off my asking the submitter as to why she preferred the approach rather than assuming that your approach is the better way.

9. If you think the submitter is not clear about what you are trying to convey, take the initiative and say “let me know if you need help”

10. Have fun as a reviewer. There’s always something to learn from someone.

As a submitter: 

1. Please give your code  a thought, the reviewer is taking time out to help you better YOUR code.

2. When the reviewer suggests something or files a bug, try not to take it personally!

3. if you think some change is not required or can be held off in another CL, start a discussion and state your reasons clearly.

4. Involve the reviewer, if you are unsure about something, take the initiative and ask her what she thinks about it. Remember reviews are about collaboration.

5. Don’t forget to have fun. Remember its your code. Make it pretty, make it functional, also keep it clean.

Me: Wow look at those patents.

Co-worker: Makes it all feel so impersonal.

 

 

Agile process.

I moved to  small-medium sized company from a large one. And have learnt some lessons in the past month or so. The company I joined was primarily water fall, but now we are moving to be more agile. The team that I work with, has been agile for a few years now, and well function as their own little unit.

At the big company, in my team things were already in place, well most of it. There were tweaks done to teams, for example, there was no formal walk through of the requirement with the QA, there was no code overview of the dev with the QA, the dev and QA mostly worked with, on demand. As in when the QA asked for information, it was provided.There were no test case reviews. What was definitely there was adherence to feature complete, code complete and site complete dates. The dates became stringent as the product evolved, that is  code and site complete was important, even if it meant that the feature complete dates were missed.There was weight-age given to high priority bugs.

This team has around 5-6 developers including the manager, who is very hands on. Yes the manager codes, and manages. When I say manage- I mean manages the projects, assigns tasks. But I also see that he does not manage people- or maybe he does- but he treats them like adults, and the developers behave like adults. They finish up tasks assigned to them. If they need help, they raise up their hands and ask for help.I like the team, most of the communication flows through the manager- but, the product and the QA are free to communicate with them directly. But the manager is always informed when a fix has a problem, or if the dev is stumped for a solution. The manager is the most experienced dev on the team- hence he also behaves as the architect.

The product managers are by far the best ever I have worked with. The are on top of it. The tool followed here is Rally, which means each and every use case has been transformed to user stories, with presentations and images- for the QA to view. The QA is however not involved in the requirements, this comes from the product.marketing team. The test cases are written for each user story, sent for review – to the product manager and the developer who is coding the user story- not the lead, not the manager. I love it that the developer and mainly the product manager takes time to review it- it usually goes through several cycles before its perfected. So far since there are two qa’s on the team, its feels awesome that as the code is being developed, the test cases are being developed, and when the code is finished, the QA is aware what the functionality is, and is ready to start testing them.

The product manager gives the QA a list of user stories that the QA would need to write up at the start of the sprint, and is expected to finish up the testcases and test if possible at teh end of the sprints.

What I like about the team, and find it a challenge, is that the product manager is so knowledgeble, finding bugs before the QA does. It challenges me, to get really picky, and to watch out for bugs. In conclusion I think I would not be too wrong if I said that how a agile process works depends on how good a product/project manager is.

OK coming to what I don;t like- when we initially started testing, I was told that they prefer that we communicate the issues before converting them to bugs. I was not comfortabel with that, but now that I see how the team works, I think its nice- saves us time and the developers are very forth coming in telling us how much of coding is left, anf that ultimately its pointless fo rus to file those bugs. Miracle of miracles, they seem to be doing alot of unit testing and self testing- which makes it harder for us to find bugs.

I also dno’t liek it that the process is fluid, no one talks about feature complte dates. no one talked about code compelte. Its very fluid- I have not yeat released a product with this team, so I’ll hold on to my negativity and see how it flows.

LIke I mentioned earlier we use Rally, I think its a handy tool and gives a good overview of user story generation to progress in test executions, its also possible to file bugs via rally. We havent experimented with that yet.

Finally, I think I understadn what agile process means. its means fludity in the process, its about people on the team coming together, and expressing what works and doesn’t work for them, and each member of the team is a player- everyone has to be actively involved- it assumes your team is mature, and dedicated to the work they are into.

Also the daily scrum is really agile, we meet in a white walled room, with a glass wall- and a telephone hung on to the wall. No furniture what so ever, we stand around the room and tell people what we worked on. 10 minutes tops. No computer, no presentation – nothing. I like it- some hate it. They think its pointless. Coming from primarily an agile process,  I love it- I know what the devs were up to- and plan on working- and I can voice by blocks – so everyone on the team is aware of it.

Perl string manipulations when specified as a scalar.

When one defines a scalar variable, and then assigns a string type to it, it can get confusing.At least in my mind, its one huge blob of data ,so how does one say for example calculate the number of characters in it?, or manipulate it?. My first instinct was to compare $_ with the any character in a foreach. as in


foreach ($somestring){

if($_  ne undefined){

....do something;

}

}

Till I printed out the data and realised that $_ will see the string(scalar) variable as one blob of data.

Here is a tiny easy breezy program:

Create a scalar variable containing the string “The quick brown fox jumps over the lazy dog”.Print out the length of this string, and then using substr, print out the fourth word (fox).   Update this to Replace the word “fox” in the above string with “kitten”.


#! usr/bin/perl -w

use strict;

my $sentence="The quick brown fox jumps over the lazy dog";

my $length=0;

$length=length($sentence);
 print "The total number of characters in $sentence is $length \n";

substr ($sentence,15,4)=" kitten";

print " $sentence

There is a handy length inbuilt function to do the trick, and strings can easily be manipulated using the format here:

However a point to note is that if you try and substitute substr ($sentence,15,4)=” kitten”; with $somestring=substr($sentence,15,4,”kitten”) didn’t seem to work. I don’t know why, perl version I am using?. Let me know if you know why, else I’ll update this post once I figure it out 🙂

Happy coding.