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

Timestamp:

2015-02-12T16:56:57+01:00 (9 years ago)

Author:

weaver

Comment:

--

ticket/1327/review

@@ -4 +4 @@
 
 == For completion by the Sci/Tech/Code reviewer ==
-'''Reviewer:''' [ Enter your name here ]
+'''Reviewer:''' [ Anthony Weaver ]
 
 === Ticket Details, Documentation and Code changes ===
-||Do you understand the area of code being altered and the reasoning why it is being altered?||YES/NO||
-||Do the proposed code changes correspond with the stated reason for the change?||YES/NO||
-||Is the in-line documentation accurate and sufficient?||YES/NO||
-||Do the code changes comply with NEMO coding standards?||YES/NO||
-||Is the Ticket documented with sufficient detail for others to understand the impact of the change?||YES/NO||
-||Does any corresponding external documentation require updating?||YES/NO||
-||If yes, which docs and have the updates been drafted?||YES/NO||
-||Are namelist changes required for this change?||YES/NO||
-||If yes, have they been done?||YES/NO||
-||Has a completed Ticket Summary template been appended to the ticket to aid code reviews||YES/NO||
-||Does this summary correspond with your understanding of the full ticket?||YES/NO||
+||Do you understand the area of code being altered and the reasoning why it is being altered?||YES||
+||Do the proposed code changes correspond with the stated reason for the change?||YES||
+||Is the in-line documentation accurate and sufficient?||YES||
+||Do the code changes comply with NEMO coding standards?||NOT ENTIRELY||
+||Is the Ticket documented with sufficient detail for others to understand the impact of the change?||YES||
+||Does any corresponding external documentation require updating?||YES||
+||If yes, which docs and have the updates been drafted?||OBS, not sure||
+||Are namelist changes required for this change?||NO||
+||If yes, have they been done?||NA||
+||Has a completed Ticket Summary template been appended to the ticket to aid code reviews||YES||
+||Does this summary correspond with your understanding of the full ticket?||YES||
 
 Ticket, Documentation and Code comments
@@ -24 +24 @@
 
 === Testing ===
-||Has the NVTK and other jobs been tested with this change?||YES/NO||
-||Have the required bit comparability tests been run?||YES/NO||
-||Can this change be shown to have a null impact? (if option not selected)||YES/NO||
-||If no, is reason for the change valid/understood?||YES/NO/NA||
-||If no, ensure that the ticket details the impact this change will have on model configurations .||YES/NO/NA||
-||Is this change expected to preserve all diagnostics?||YES/NO||
-||If no, is reason for the change valid/understood?||YES/NO/NA||
-||Are there significant changes in run time/memory?||YES/NO||
+||Has the NVTK and other jobs been tested with this change?||YES||
+||Have the required bit comparability tests been run?||YES||
+||Can this change be shown to have a null impact? (if option not selected)||YES||
+||If no, is reason for the change valid/understood?||NA||
+||If no, ensure that the ticket details the impact this change will have on model configurations .||NA||
+||Is this change expected to preserve all diagnostics?||NO||
+||If no, is reason for the change valid/understood?||YES||
+||Are there significant changes in run time/memory?||YES||
 
 Testing Comments
@@ -37 +37 @@
 Add specific testing comments here
 
-Add specific testing comments here
 
 === Code Review ===
-||Do the code changes comply with NEMO coding standards?||YES/NO||
-||Are code changes consistent with the design of NEMO?||YES/NO||
-||Is the code free of unwanted TABs?||YES/NO||
-||Has the code been wholly (100%) produced by NEMO developers working on NEMO?||YES/NO||
+||Do the code changes comply with NEMO coding standards?||NOT ENTIRELY||
+||Are code changes consistent with the design of NEMO?||YES||
+||Is the code free of unwanted TABs?||HAVE NOT CHECKED||
+||Has the code been wholly (100%) produced by NEMO developers working on NEMO?||YES||
 ||If no, ensure collaboration agreement has been added to the ticket keywords||||
 
 Add specific code comments or suggested alterations here.
+- Fix various alignment and indentation issues (eg, & for line continuation
+  should be 3 spaces in from previous line)
+- Group the local declarations by same type.
+- Separate the Argument and local declarations by a space
+- The local INTEGER array v_indic uses a naming convention for global real 
+  variables. The array name should start with an "i".
 
 === Review Summary ===
-Add summary here
+The interpolation in the observation operators has been made available with
+s-coordinates. This work has essentially involved reversing the order of the
+horizontal-vertical interpolation compared to what is done in z-coordinates.
+The code has been successfully tested.
 
 === Approval for the trunk ===
-YES/NO
+YES, after fixing the minor issues with code format above.
 
 The code reviewer may approve the change for the NEMO trunk when: