Version 3 (modified by bouttier, 5 years ago) (diff)

Last edited Timestamp?

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.
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.
  2. the above check-list has been completed.

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