14420
Comment:
|
16585
|
Deletions are marked like this. | Additions are marked like this. |
Line 38: | Line 38: |
1. Review participants are selected, usually only two people. 1. The implementor specifies the code to be reviewed in the [:Code Review Process#CodeReviewOverview:Code Review Overview per Iteration]. This includes line numbers and SVN version for each file. |
1. Review participants are specified in the current [Iteration task list]. Usually it is the implementor and another developer. 1. The implementor specifies the code to be reviewed in Crucible 1. Log on to Crucible (http://kb-prod-udv-001.kb.dk:8060) 1. Go to your Crucible !DashBoard (using the link "My Crucible !DashBoard") 1. Click on "Create new review" in the top right corner 1. Select a proper titel (e.g. Feature request Y, og Bug Y plus description). If it corresponds to a task in the current [Iteration task list], it should have the same title, as is written there. 1. Write relevant comments in the "Statement of Objects" for instance genral comments to the review and list of deleted file or class names that for obvious reasons cannot be included. 1. Add the files included in the review. This can be done by selecting files in different changesets or in the repository????. NOTE that we do not review code in the test-branch. 1. Add a revision comment to each of the files with specific lines to be review (or ALL LINES). Also note what happened here if relevant e.g. lines that have been removed (fil/class name and revision of file is given automatically by Crucible) 1. Make an entry with review information for the review in the wiki current [Review Table] (see for example ????) .... 1. You notify the reviewer of the existence of a review, by clicking "Start Review". |
Line 41: | Line 50: |
1. The implementor sets up or extends the [:Code Review Process#CodeReviewPagePerPage:Code Review Page per Class/JSP-page] for each file, noting date, version and areas covered. 1. Before the review time, each participant reads the code thoroughly, noting problems that should be discussed. These problems range from lack of white space around delimiters to serious bugs or design problems. |
1. Before the review time, each participant reads the code thoroughly and note problems - big or small - that should be discussed on the relevant places in Crucible. For instance: * Add a new general comment: [[BR]] For general problems like design problems or problems concerning most of the files like missing '.' in end of !JavaDoc. * Add a revision comment: [[BR]] For general problems in a specific file like generally missing !JavaDoc in this particular file. * On specific line (mark line will result in a comment field to appear): [[BR]] For problems on specific lines of a file like lack of white space around delimiters. .'''REMEMBER" to post the comments by clicking "Post" for each of the comments. * The reviewer can now start examinining the files under review, and contact the review creator concerning the time for the review Before we start, you need to have an crucible account. If you haven't yet acquired an account, you can sign up for one. It is crucial, that your username is the same as the one you are using to commit to subversion, because you can only setup reviews for code, that you have committed yourself. However, you can select any display name, you like. [Note: That is why I have created a dummy account for KFC with the display name 'På vegne af KFC' to be able to create reviews for his yet unreviewed code.] |
The Code Review process covers:
[#CodeReviewPurpose Purpose]
[#CodeReviewResponsible Responsible]
[#CodeReviewMethod Method]
[#CodeReviewPlanning Planning]
[#CodeReviewReviewSession Review Session]
[#CodeReviewFollowUp Follow-Up]
[#CodeReviewTime Time]
[#CodeReviewInput Input]
[#CodeReviewOutput Output]
[#CodeReviewBackground Background for Code Review process]
[#CodeReviewResourceUsage Resource Usage]
[#CodeReviewLiterature Literature]
Anchor(CodeReviewPurpose) PurposeBR We use code reviews to improve correctness and stability of our code. The main purposes of code reviews are traditionally
- to reduce the overall error rate
- to improve productivity
Another equally important effect of code-reviews is
- to distribute information about the code and establish common coding practices
Anchor(CodeReviewResponsible) ResponsibleBR The Task holder of [:Process/implementation:implementation] task which for example can be the [:Process_Role/Module_Owner:Module Owner] for small corrections or another assigned developer.
Anchor(CodeReviewMethod) MethodBR A code review consists of 2 or more persons that read the code and in a structured way identify changes that will improve the overall quality of the code. Unit test are not usually included. Code review is the third phase of implementation (following unit test writing and implementation). It is normally done when the relevant part of the code is fully implemented, i.e. fulfills all the unit tests, has been sanity tested and documented. Our process for code review contain planning, review session and follow-up as described below.
Anchor(CodeReviewPlanning) Planning
- Review participants are specified in the current [Iteration task list]. Usually it is the implementor and another developer.
- The implementor specifies the code to be reviewed in Crucible
Log on to Crucible (http://kb-prod-udv-001.kb.dk:8060)
Go to your Crucible DashBoard (using the link "My Crucible DashBoard")
- Click on "Create new review" in the top right corner
- Select a proper titel (e.g. Feature request Y, og Bug Y plus description). If it corresponds to a task in the current [Iteration task list], it should have the same title, as is written there.
- Write relevant comments in the "Statement of Objects" for instance genral comments to the review and list of deleted file or class names that for obvious reasons cannot be included.
- Add the files included in the review. This can be done by selecting files in different changesets or in the repository????. NOTE that we do not review code in the test-branch.
- Add a revision comment to each of the files with specific lines to be review (or ALL LINES). Also note what happened here if relevant e.g. lines that have been removed (fil/class name and revision of file is given automatically by Crucible)
- Make an entry with review information for the review in the wiki current [Review Table] (see for example ????) ....
- You notify the reviewer of the existence of a review, by clicking "Start Review".
- The participants agree on a time to review.
- Before the review time, each participant reads the code thoroughly and note problems - big or small - that should be discussed on the relevant places in Crucible. For instance:
Add a new general comment: BR For general problems like design problems or problems concerning most of the files like missing '.' in end of JavaDoc.
Add a revision comment: BR For general problems in a specific file like generally missing JavaDoc in this particular file.
On specific line (mark line will result in a comment field to appear): BR For problems on specific lines of a file like lack of white space around delimiters.
REMEMBER" to post the comments by clicking "Post" for each of the comments.
- The reviewer can now start examinining the files under review, and contact the review creator concerning the time for the review
Before we start, you need to have an crucible account. If you haven't yet acquired an account, you can sign up for one. It is crucial, that your username is the same as the one you are using to commit to subversion, because you can only setup reviews for code, that you have committed yourself. However, you can select any display name, you like. [Note: That is why I have created a dummy account for KFC with the display name 'På vegne af KFC' to be able to create reviews for his yet unreviewed code.]
Anchor(CodeReviewReviewSession) Review SessionBR This part, while central to the whole process, should not be allowed to drag on forever. If the reviewers cannot agree on how to fix a problem within a few minutes, the item should be marked as "consider how to..." rather than prolonging the discussion. A typical review session should take no more than an hour (and most take less than that). If it takes longer, the review should be stopped and a time to continue should be agreed upon. More than an hour of straight review reduces the efficiency. Discuss each code item in order of appearance in the file. Those items that the participants don't agree to discard are noted by line number and with a severity tag on the [:Code Review Process#TablesFilledPageReview:Code Review Tables filled for each review of a class/JSP-page] If flaws are found during code review, one or more persons must at the end of review be chosen to follow up on the flaws found. Usually, this should be the one who made the changes in the first place, but if the changes are large or other time constraints interfere, one of the other reviewers can step on. The choice must be noted in the [:Code Review Process#TableFilledReviewsOverview:Code Review Overview Table Filled for each Iteration]. There are two kinds of review pages:
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review Each class/JPS-page has its own page with all code reviews and their documentation made on the specific Class/JPS-page.
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented Each code review page is named according to the codes position in the Java project. For classes the name is formed from the class and package name as follows <Unique package name for class> + <Class name> + "Review" Where each part of the name starts by upper case and continuous in lower case (as WikiWords), for example For JSP pages the name is formed from the JSP-page group and the JSP page name as follows <Unique group for JSP-page > + <JSP-page name> + "JSPReview" Where each part is of the name starts by upper case and continuous in lower case – and “-“ are skipped where letter after “-“ is written in uppercase too (as WikiWords), for example HistoryHarveststatusJobdetailsJSPReview for History/!HarvestStatus-jobdetails.jsp (under /trunk/webpages/) Anchor(TablesFilledPageReview)
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review The below tables (from [:CodeReviewCodePageTemplate:template]) keeps the information for each review of a class/JSP-page (or parts of one). If a class/JSP-page is reviewed more than once, new sections like this get added at the top of the same page. Storing the old reviews with task, date, SVN version and lines has proven useful for tracking down problematic changes and misunderstood designs. Example is CommonDistributeChannelsReview Anchor(CreationCodeReviewCodePage)
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review You must use the CodeReviewCodePageTemplate (Code Review Class/JSP Page Template) to create a new page. If an old review page exists on another media then this link should be referenced.
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review If the Code Review Class Page already exists then the tables for a new review is inserted in the top of the page in order always to see newest review text first. The page may contain a link to old review pages which is placed on another media and therefore not readable for all.
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review Each iteration has its own page with an overview of code reviews, author of changes and who the reviewer is. Anchor(NameCodeReviewOverview)
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review Each Iteration review overview page is named according to the Iteration name. The name is formed from the iteration number as follows "Iteration" + <Iteration number> + "ReviewsOverview" for example Anchor(TableFilledReviewsOverview)
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review The below table (from [:CodeReviewOverviewPageTemplate:template]) keeps information of reviews made on a class/JSP-page within an Iteration. Example is Iteration33ReviewsOverview Anchor(CreationCodeReviewOverviewPage)
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review You must use the CodeReviewOverviewPageTemplate(Code Review Iteration Page Template) to create a new page. Anchor(UpdateCodeReviewOverviewPage)
Note this is taken from the current process, it will be changed according to use of a new tool for registration of code reviews is chosen and implemented BR The contents can be seen as guidence of what informaiton should be through the review For each class/JSP-page to be reviewed, there must be added a table line describing it. Note that the same class/JSP-page may appear several times. but depending on the implemented task, it can also be corrected documentation scripts etc. Anchor(CodeReviewResourceUsage) Steve McConnell, Rapid Development page 73-74 [http://satc.gsfc.nasa.gov/fi/fipage.html:NASA's ideas for code inspection]
Follow-up Review Pages (technical information)
Code Review Page per Class/JSP-page
Name of Code Review Class/JSP page
Code Review Tables filled for each review of a class/JSP-page
Creation of New Code Review Class/JSP Page
Update of Existing Code Review Class/JSP Page
Code Review Overview per Iteration
Name of Iteration Code Review Overview
Code Review Overview Table Filled for each Iteration
Creation of New Iteration Code Review Overview Page
Update of Existing Iteration Code Review Overview Page