Changes between Version 1 and Version 2 of ticket/1327/review


Ignore:
Timestamp:
2015-02-12T16:56:57+01:00 (6 years ago)
Author:
weaver
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • ticket/1327/review

    v1 v2  
    44 
    55== For completion by the Sci/Tech/Code reviewer == 
    6 '''Reviewer:''' [ Enter your name here ] 
     6'''Reviewer:''' [ Anthony Weaver ] 
    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/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|| 
     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?||NOT ENTIRELY|| 
     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?||OBS, not sure|| 
     16||Are namelist changes required for this change?||NO|| 
     17||If yes, have they been done?||NA|| 
     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/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|| 
     26||Has the NVTK and other jobs been tested with this change?||YES|| 
     27||Have the required bit comparability tests been run?||YES|| 
     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?||NA|| 
     30||If no, ensure that the ticket details the impact this change will have on model configurations .||NA|| 
     31||Is this change expected to preserve all diagnostics?||NO|| 
     32||If no, is reason for the change valid/understood?||YES|| 
     33||Are there significant changes in run time/memory?||YES|| 
    3434 
    3535Testing Comments 
     
    3737Add specific testing comments here 
    3838 
    39 Add specific testing comments here 
    4039 
    4140=== 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|| 
     41||Do the code changes comply with NEMO coding standards?||NOT ENTIRELY|| 
     42||Are code changes consistent with the design of NEMO?||YES|| 
     43||Is the code free of unwanted TABs?||HAVE NOT CHECKED|| 
     44||Has the code been wholly (100%) produced by NEMO developers working on NEMO?||YES|| 
    4645||If no, ensure collaboration agreement has been added to the ticket keywords|||| 
    4746 
    4847Add specific code comments or suggested alterations here. 
     48- Fix various alignment and indentation issues (eg, & for line continuation 
     49  should be 3 spaces in from previous line) 
     50- Group the local declarations by same type. 
     51- Separate the Argument and local declarations by a space 
     52- The local INTEGER array v_indic uses a naming convention for global real  
     53  variables. The array name should start with an "i". 
    4954 
    5055=== Review Summary === 
    51 Add summary here 
     56The interpolation in the observation operators has been made available with 
     57s-coordinates. This work has essentially involved reversing the order of the 
     58horizontal-vertical interpolation compared to what is done in z-coordinates. 
     59The code has been successfully tested. 
    5260 
    5361=== Approval for the trunk === 
    54 YES/NO 
     62YES, after fixing the minor issues with code format above. 
    5563 
    5664The code reviewer may approve the change for the NEMO trunk when: