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/1085/Review (diff) – NEMO

Changes between Version 2 and Version 3 of ticket/1085/Review


Ignore:
Timestamp:
2013-11-14T12:26:44+01:00 (10 years ago)
Author:
vichi
Comment:

--

Legend:

Unmodified
Added
Removed
Modified

  • ticket/1085/Review

    v2 v3  
    44 
    55== For completion by the Sci/Tech/Code reviewer == 
    6 '''Reviewer:''' [ Enter your name here ] 
     6'''Reviewer:''' MARCELLO VICHI 
    77 
    88=== 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'''|| 
    17 ||If yes, have they been done?||'''YES'''|| 
    18 ||Has a completed Ticket Summary template been appended to the ticket to aid code reviews||'''YES'''|| 
    19 ||Does this summary correspond with your understanding of the full ticket?||YES/NO|| 
     9|| Do you understand the area of code being altered and the reasoning why it is being altered? || YES || 
     10|| Do the proposed code changes correspond with the stated reason for the change? || YES || 
     11|| Is the in-line documentation accurate and sufficient? || YES || 
     12|| Do the code changes comply with NEMO coding standards? || YES || 
     13|| Is the Ticket documented with sufficient detail for others to understand the impact of the change? || YES || 
     14|| Does any corresponding external documentation require updating? || YES || 
     15|| If yes, which docs and have the updates been drafted? || ONGOING || 
     16|| Are namelist changes required for this change? || '''YES''' || 
     17|| If yes, have they been done? || '''YES''' || 
     18|| Has a completed Ticket Summary template been appended to the ticket to aid code reviews || '''YES''' || 
     19|| Does this summary correspond with your understanding of the full ticket? || YES || 
    2020 
    2121Ticket, Documentation and Code comments 
    … …  
    2424 
    2525=== 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'''|| 
    32 ||If no, is reason for the change valid/understood?||'''N/A'''|| 
    33 ||Are there significant changes in run time/memory?||'''NO'''|| 
     26|| Has the NVTK and other jobs been tested with this change? || YES || 
     27|| Have the required bit comparability tests been run? || NO || 
     28|| Can this change be shown to have a null impact? (if option not selected) || YES || 
     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''' || 
     32|| If no, is reason for the change valid/understood? || '''N/A''' || 
     33|| Are there significant changes in run time/memory? || '''NO''' || 
    3434 
    3535Testing Comments 
    … …  
    3838 
    3939=== Code Review === 
    40 ||Do the code changes comply with NEMO coding standards?||YES/NO|| 
    41 ||Are code changes consistent with the design of NEMO?||YES/NO|| 
    42 ||Is the code free of unwanted TABs?||YES/NO|| 
    43 ||Has the code been wholly (100%) produced by NEMO developers working on NEMO?||'''YES'''|| 
     40|| Do the code changes comply with NEMO coding standards? || YES || 
     41|| Are code changes consistent with the design of NEMO? || YES || 
     42|| Is the code free of unwanted TABs? || YES || 
     43|| Has the code been wholly (100%) produced by NEMO developers working on NEMO? || '''YES''' || 
    4444 
    4545Add specific code comments or suggested alterations here. 
    4646 
    4747=== Review Summary === 
    48 Add summary here 
     48The changes represent a necessary update of the C1D option that remained silent for some time. It is now a usable option that should be integrated as soon as possible with the other development for 1D cases done by Mercator. As an added value, now fldread can also deal with 0-D input files (i.e. timeseries) which may also be useful for other cases. 
    4949 
    5050=== Approval for the trunk === 
    51 YES/NO 
     51YES 
    5252 
    5353The code reviewer may approve the change for the NEMO trunk when: