Last edited [[Timestamp]]

[[PageOutline]]

== For completion by the Sci/Tech/Code reviewer ==
'''Reviewer 1:''' Pierre-Antoine Bouttier

=== Ticket Details, Documentation and Code changes ===
|| Do you understand the area of code being altered and the reasoning why it is being altered? || YES || YES/NO ||
|| Do the proposed code changes correspond with the stated reason for the change? || YES || YES/NO ||
|| Is the in-line documentation accurate and sufficient? || YES || YES/NO ||
|| Do the code changes comply with NEMO coding standards? || YES || YES/NO ||
|| Is the Ticket documented with sufficient detail for others to understand the impact of the change? || YES || YES/NO ||
|| Does any corresponding external documentation require updating? || YES || YES/NO ||
|| If yes, which docs and have the updates been drafted? || The OBS chapter in the NEMO book has been drafted. || YES/NO ||
|| Are namelist changes required for this change? || YES || YES/NO ||
|| If yes, have they been done? || YES || YES/NO ||
|| Has a completed Ticket Summary template been appended to the ticket to aid code reviews || YES || YES/NO ||
|| Does this summary correspond with your understanding of the full ticket? || YES || YES/NO ||

Ticket, Documentation and Code comments

'''Reviewer 1''': No specific comments on the documentation.

=== Testing ===
|| Has the NVTK and other jobs been tested with this change? || YES || YES/NO ||
|| Have the required bit comparability tests been run? || YES || YES/NO ||
|| Can this change be shown to have a null impact? (if option not selected) || YES || YES/NO ||
|| If no, is reason for the change valid/understood? || NA || YES/NO/NA ||
|| If no, ensure that the ticket details the impact this change will have on model configurations . || NA || YES/NO/NA ||
|| Is this change expected to preserve all diagnostics? || YES || YES/NO ||
|| If no, is reason for the change valid/understood? || NA || YES/NO/NA ||
|| Are there significant changes in run time/memory? || NO || YES/NO ||

Testing Comments

'''Reviewer 1''': The proposed SETTE tests give as outputs the same statistics in ocean.ouput than the previous OBS version on my own machines. 

Add specific testing comments here

=== Code Review ===
|| Do the code changes comply with NEMO coding standards? || YES || YES/NO ||
|| Are code changes consistent with the design of NEMO? || YES || YES/NO ||
|| Is the code free of unwanted TABs? || YES || YES/NO ||
|| Has the code been wholly (100%) produced by NEMO developers working on NEMO? || YES || YES/NO ||
|| If no, ensure collaboration agreement has been added to the ticket keywords ||

'''Reviewer 1''': I have one suggestion, more about the NEMO architecture. It seems to me more meaningful to extract the OBS module from the OPA_SRC directory.[[BR]]First, for me, it is not directly related to the NEMO dynamic core, then it can be used by other NEMO components (Sea-Ice component, for example)

=== Review Summary ===
'''Reviewer 1''': These changes correspond clearly to a very welcomed simplification of the OBS module. They are well written, both for code and documentation, according to the NEMO coding conventions.

=== Approval for the trunk ===
'''Reviewer 1''': YES Reviewer 2: YES/NO

The code reviewer may approve the change for the NEMO trunk when:

 1. their requests/comments have been addressed satisfactorily.
 1. the above check-list has been completed.

or the code reviewer may choose to reject & assign the change back to the code author.