[pisa-src] Re: IMPORTANT: Code review scheduled for week 2.11. - 6.11.

  • From: Thomas Jansen <mithi@xxxxxxxxx>
  • To: pisa-src@xxxxxxxxxxxxx
  • Date: Fri, 23 Oct 2009 14:25:04 +0200

On Thu, Oct 22, 2009 at 03:14:47PM +0200, René Hummen wrote:

For all the text in the mail, it's a rather vague description of what's
actually planned.

> Therefore and in order to get a better understanding of the codebase for 
> the new project members, I am scheduling a code review for the whole week 
> from 2.11. - 6.11.

A whole week? Who do you consider "project members" for the code review?
If that includes the average 10h/week HiWi, then 'a week' is quite a
euphemism. Especially for the HiWis it's difficult to agree on a time when
everyone has to be present.

> Diego and Thomas have already been doing a great job at cleaning up the 
> code for the last 1 - 2 weeks, so maybe we might just find out that it is 
> in a perfect shape by now.

True, we've been cleaning up tons of things that have been around for ages
without anyone wondering why or how it really works. Still we both have some
tasks left before we can call it "done" (or rather a reasonable starting point
for further development).

The current build system with autotools and recursive make is a known weakness
and Diego has been on that task for quite a while. I'm currently focussing on
general architecture of the performance-critical sections (e.g. writing a new
scheduler to improve throughput). A shared task was (and to a small extent
still is) getting rid of compiler warnings in the code.

> But I am quite confident that we can improve the code even further altogether
> and that we can make it more stable and "smart" than it is right now.

Smart as in additional functionality or as in clever way to do what's already
being done clumsily?

> So, if you stumbled across some piece of code during your development that
> really made your hair stand on end, this phase will be the right time to
> mention and fix that :-)

There are a bunch of people who never touched the source and those who only
looked at the specific parts they needed to change. I'd claim that there is
currently no person in the project that knows *all* of the code.

> The plan is to meet up Monday morning (2.11.2009) at 10:00 to shortly  
> discuss about issues that need to be resolved and parts of the code  
> worth inspecting. We then assign tasks and spread up to get the work  
> done.

So we spend an hour or so to make a todo list. What's planned for the rest of
the week? Do the tasks refer to fixing the items we put on the list?

As for parts of the code that are worth inspecting: IMO pisa{s,c}d are the
heavy duty binaries that need to be stable and efficient, so they need the
most attention in combination with libpisa (that both use).

I mark pieces of code that need improvement with /* TODO */ comments, perhaps
others do something similar. Doing a simple grep probably yields enough work
for a day or two.

So I guess my main question is: What's the purpose of the code review and how
exactly should it happen? As there are still three weeks left, perhaps some
preparations should be made, like everyone making a list of things they want
to discuss?

-- 
Thomas Jansen, "Mithi" --- mithi@xxxxxxxxx
GPG 9D5C682B, feel free to sign or encrypt your mail

Other related posts: