New URL for NEMO forge!   http://forge.nemo-ocean.eu

Since March 2022 along with NEMO 4.2 release, the code development moved to a self-hosted GitLab.
This present forge is now archived and remained online for history.
ticket/0718/Review (diff) – NEMO

Changes between Version 1 and Version 2 of ticket/0718/Review


Ignore:
Timestamp:
2010-10-05T16:15:38+02:00 (14 years ago)
Author:
rblod
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • ticket/0718/Review

    v1 v2  
    44 
    55== For completion by the Sci/Tech/Code reviewer == 
    6 '''Reviewer:''' [ RAchid Benshila] 
     6'''Reviewer:''' Rachid Benshila[[BR]] 
     7 
     8 * basically nothing to improve in term of implementation[[BR]] 
     9 * style off additions is ok, some changes to do in term of cosmetic in old bdy routines (see attached routines) [[BR]] 
     10 * general observation : we should use one array being either  bdytmask either obctmask, it would avoid some duplication (solmat, sshwzw)[[BR]] 
     11 * general observation : we should use fldread for bdydta (and obcdta) 
    712 
    813=== Ticket Details, Documentation and Code changes === 
    9 ||Do you understand the area of code being altered and the reasoning why it is being altered?||YES/NO|| 
    10 ||Do the proposed code changes correspond with the stated reason for the change?||YES/NO|| 
    11 ||Is the in-line documentation accurate and sufficient?||YES/NO|| 
    12 ||Do the code changes comply with NEMO coding standards?||YES/NO|| 
    13 ||Is the Ticket documented with sufficient detail for others to understand the impact of the change?||YES/NO|| 
    14 ||Does any corresponding external documentation require updating?||YES/NO|| 
    15 ||If yes, which docs and have the updates been drafted?||YES/NO|| 
    16 ||Are namelist changes required for this change?||YES/NO|| 
    17 ||If yes, have they been done?||YES/NO|| 
    18 ||Has a completed Ticket Summary template been appended to the ticket to aid code reviews||YES/NO|| 
    19 ||Does this summary correspond with your understanding of the full ticket?||YES/NO|| 
     14||Do you understand the area of code being altered and the reasoning why it is being altered?||YES|| 
     15||Do the proposed code changes correspond with the stated reason for the change?||YES|| 
     16||Is the in-line documentation accurate and sufficient?||YES|| 
     17||Do the code changes comply with NEMO coding standards?||YES|| 
     18||Is the Ticket documented with sufficient detail for others to understand the impact of the change?||YES|| 
     19||Does any corresponding external documentation require updating?||YES|| 
     20||If yes, which docs and have the updates been drafted?||is underway|| 
     21||Are namelist changes required for this change?||YES|| 
     22||If yes, have they been done?||NA|| 
     23||Has a completed Ticket Summary template been appended to the ticket to aid code reviews||YES|| 
     24||Does this summary correspond with your understanding of the full ticket?||YES|| 
     25 
     26 
    2027 
    2128Ticket, Documentation and Code comments 
     
    2431 
    2532=== Testing === 
    26 ||Has the NVTK and other jobs been tested with this change?||YES/NO|| 
    27 ||Have the required bit comparability tests been run?||YES/NO|| 
    28 ||Can this change be shown to have a null impact? (if option not selected)||YES/NO|| 
    29 ||If no, is reason for the change valid/understood?||YES/NO/NA|| 
    30 ||If no, ensure that the ticket details the impact this change will have on model configurations .||YES/NO/NA|| 
    31 ||Is this change expected to preserve all diagnostics?||YES/NO|| 
    32 ||If no, is reason for the change valid/understood?||YES/NO/NA|| 
    33 ||Are there significant changes in run time/memory?||YES/NO|| 
     33||Has the NVTK and other jobs been tested with this change?||NA|| 
     34||Have the required bit comparability tests been run?||NA|| 
     35||Can this change be shown to have a null impact? (if option not selected)||YES|| 
     36||If no, is reason for the change valid/understood?||NA|| 
     37||If no, ensure that the ticket details the impact this change will have on model configurations .||NA|| 
     38||Is this change expected to preserve all diagnostics?||NO|| 
     39||If no, is reason for the change valid/understood?||YES|| 
     40||Are there significant changes in run time/memory?||NO|| 
    3441 
    3542Testing Comments 
     
    4047 
    4148=== Code Review === 
    42 ||Do the code changes comply with NEMO coding standards?||YES/NO|| 
    43 ||Are code changes consistent with the design of NEMO?||YES/NO|| 
    44 ||Is the code free of unwanted TABs?||YES/NO|| 
    45 ||Has the code been wholly (100%) produced by NEMO developers working on NEMO?||YES/NO|| 
     49||Do the code changes comply with NEMO coding standards?||YES|| 
     50||Are code changes consistent with the design of NEMO?||YES|| 
     51||Is the code free of unwanted TABs?||YES|| 
     52||Has the code been wholly (100%) produced by NEMO developers working on NEMO?||YES|| 
    4653||If no, ensure collaboration agreement has been added to the ticket keywords|||| 
    4754 
     
    5259 
    5360=== Approval for the trunk === 
    54 YES/NO 
     61YES (waiting for the documentation) 
    5562 
    5663The code reviewer may approve the change for the NEMO trunk when: