'. '

APIReview

From APIDesign

(Difference between revisions)
Jump to: navigation, search
Line 1: Line 1:
API Review is an essential element of [[Teamwork]] for everyone designing APIs in a group. The [[NetBeans]] project uses one described at [[netbeans:APIReviews]].
API Review is an essential element of [[Teamwork]] for everyone designing APIs in a group. The [[NetBeans]] project uses one described at [[netbeans:APIReviews]].
 +
 +
== Pitfalls of API Reviews ==
 +
 +
There are two extreme position one should avoid when performing API or architecture review.
 +
There is too little to review or there is too much to review.
 +
 +
==== Show me the Code! ====
 +
 +
When proposing small change or enhancement, it is preferrable to do it with a patch to existing
 +
codebase. As Linus Torwalds once put it: ''talk is cheap, show me the code!''
 +
 +
Often, rather than describing how you want to add one method into an existing class, it is easier
 +
to prepare the patch. Such patch should include documentation and a test to show what is the new
 +
capability of the API that wasn't available before.
 +
 +
Evaluating such patch is usually straightforward, reviewers know what to focus on and, if the
 +
documentation is [[good]] they know what is the purpose of the test.
 +
 +
Also creating such patch is usually easier than long propse sections describing of what
 +
could be done without showing how. At the end most of the prose sections should become
 +
part of the [[Javadoc]] documentation - thus one can include them in the patch as well.
 +
 +
In short, when proposing small, compatible additions to existing APIs, doing that
 +
as a patch is the best style of review. [[NetBeans]] call that '''Fast Track''' [[netbeans:APIReview]].
 +
 +
==== Don't bother me with Code! ====
 +
 +
On the other hand, some [[API]] changes can become huge. If it takes more than a day-week to code
 +
their idea, introduce completely new subsystem, change the way things use to work, it is
 +
[[good]] practise to do two rounds of view. [[NetBeans]] calls the process
 +
''Standard review'' and it is composed of ''inception'' and ''final'' review rounds.
 +
 +
The purpose of [[APIReview]]/architecture review is to review the architecture, not
 +
the actual code. However if the whole code has already been written, how can the
 +
reviewer influence the overall architecture? By commenting about wrong indentation,
 +
suggesting better variable names? Well, that isn't an architecture!
 +
 +
The danger of asking for review late is something called ''emmotional attachment to the code''.
 +
The author of the code have had spent enormous amount of time preparing it and
 +
probably put a lot of effort into its design. Any significant change in architecture
 +
basically says that the effort wasn't successful. That isn't going to make the
 +
author happy.
 +
 +
For these reasons the [[NetBeans]] recommends to start with [[UseCases]] and review them
 +
as soon as possible. Once there is an agreement on them and thus the raw direction for
 +
solving them is understood, it is much more likely the result that comes out of that
 +
is going to be generaly acceptable.
 +
 +
==== One Day Preparation ====
 +
 +
[[I]] usually try to spend about a day to prepare the review material. If [[I]]
 +
can finish the whole work, then it is eligible for ''fasttrack review'' - if not,
 +
it is probably to do deeper/standard review and as such [[I]] take what [[I]] have:
 +
[[API]] methods, their documentation and tests showing the sample usage and ask for
 +
review of such concept. Then [[I]] feel [[I]] have something material, but not that
 +
much, so the advices [[I]] can still be easily accumulated into the design.
 +
 +
Sometimes it make take longer, maybe even a week, but
 +
spending more than a week on a preparation of inception materials is too much.
 +
 +
== What if it is Too Late? ==
 +
 +
The question is what to do with reviews that qualify for a standard review and are
 +
way too big? [[I]] don't know for sure. Return back and start from scratch?
 +
Probably useless. Cherry pick the [[good]] parts and approve them first? Yeah,
 +
that is probably what [[I]] try: otherwise [[I]] am afraid that by commenting on
 +
details, the high level changes in architecture directions would be lost.
 +
 +
Wish [[I|me]] luck.

Revision as of 13:15, 17 July 2016

API Review is an essential element of Teamwork for everyone designing APIs in a group. The NetBeans project uses one described at netbeans:APIReviews.

Contents

Pitfalls of API Reviews

There are two extreme position one should avoid when performing API or architecture review. There is too little to review or there is too much to review.

Show me the Code!

When proposing small change or enhancement, it is preferrable to do it with a patch to existing codebase. As Linus Torwalds once put it: talk is cheap, show me the code!

Often, rather than describing how you want to add one method into an existing class, it is easier to prepare the patch. Such patch should include documentation and a test to show what is the new capability of the API that wasn't available before.

Evaluating such patch is usually straightforward, reviewers know what to focus on and, if the documentation is good they know what is the purpose of the test.

Also creating such patch is usually easier than long propse sections describing of what could be done without showing how. At the end most of the prose sections should become part of the Javadoc documentation - thus one can include them in the patch as well.

In short, when proposing small, compatible additions to existing APIs, doing that as a patch is the best style of review. NetBeans call that Fast Track netbeans:APIReview.

Don't bother me with Code!

On the other hand, some API changes can become huge. If it takes more than a day-week to code their idea, introduce completely new subsystem, change the way things use to work, it is good practise to do two rounds of view. NetBeans calls the process Standard review and it is composed of inception and final review rounds.

The purpose of APIReview/architecture review is to review the architecture, not the actual code. However if the whole code has already been written, how can the reviewer influence the overall architecture? By commenting about wrong indentation, suggesting better variable names? Well, that isn't an architecture!

The danger of asking for review late is something called emmotional attachment to the code. The author of the code have had spent enormous amount of time preparing it and probably put a lot of effort into its design. Any significant change in architecture basically says that the effort wasn't successful. That isn't going to make the author happy.

For these reasons the NetBeans recommends to start with UseCases and review them as soon as possible. Once there is an agreement on them and thus the raw direction for solving them is understood, it is much more likely the result that comes out of that is going to be generaly acceptable.

One Day Preparation

I usually try to spend about a day to prepare the review material. If I can finish the whole work, then it is eligible for fasttrack review - if not, it is probably to do deeper/standard review and as such I take what I have: API methods, their documentation and tests showing the sample usage and ask for review of such concept. Then I feel I have something material, but not that much, so the advices I can still be easily accumulated into the design.

Sometimes it make take longer, maybe even a week, but spending more than a week on a preparation of inception materials is too much.

What if it is Too Late?

The question is what to do with reviews that qualify for a standard review and are way too big? I don't know for sure. Return back and start from scratch? Probably useless. Cherry pick the good parts and approve them first? Yeah, that is probably what I try: otherwise I am afraid that by commenting on details, the high level changes in architecture directions would be lost.

Wish me luck.

Personal tools
buy