Changes between Version 1 and Version 2 of ticket/0859/Review

Timestamp:

2011-10-10T17:03:53+02:00 (13 years ago)

Author:

acc

Comment:

--

ticket/0859/Review

@@ -27 +27 @@
 '''namelist'''
 
-  changes are largely in place but at least one new variable is incorrectly named: ln_ascii exists in the namelists but exists as `ln_flo_ascii`     in the code. This discrepancy calls into question the level of testing since the presence of an undeclared namelist variable should cause a runtime error. Probably last minute style changes were made after testing?
+  changes are largely in place but at least one new variable is incorrectly named: ln_ascii exists in the namelists but exists as `ln_flo_ascii`          in the code. This discrepancy calls into question the level of testing since the presence of an undeclared namelist variable should cause a runtime error. Probably last minute style changes were made after testing?
 
 '''iodef.xml''' changes have been made but only to the GYRE and ORCA2_LIM configurations. Changes should be made in all cases.
@@ -45 +45 @@
 '''flodom.F90'''
 
-  Observation: We need a new code convention for namelist variables that have replaced parameters. jpnfl, jpnewflo and jpnrstflo are not parameters and therefore shouldn't start with jp. However, the same can be said of jpni, jpnj jpnij etc. so this is not the time to impose a change. To be discussed at the developer's meeting? 
-  naming convention for contained subroutines? Should they all start with flo_?: flo_add_new_floats, flo_add_new_ariane_floats, flo_findmesh, flo_dstnce 
-  Policy on allocatable vs automatic?. add_new_floats assumes automatic arrays: iimfl, ijmfl, ikmfl etc. should these be allocatable, allocated on first call and saved? 
-  constants should have the _wp precision appended (e.g. constants in dstnce) 
+  Observation: We need a new code convention for namelist variables that have replaced parameters. jpnfl, jpnewflo and jpnrstflo are not parameters and therefore shouldn't start with jp. However, the same can be said of jpni, jpnj jpnij etc. so this is not the time to impose a change. To be discussed at the developer's meeting?.
+
+  naming convention for contained subroutines? Should they all start with flo_?: flo_add_new_floats, flo_add_new_ariane_floats, flo_findmesh, flo_dstnce 
+
+  Policy on allocatable vs automatic?. add_new_floats assumes automatic arrays: iimfl, ijmfl, ikmfl etc. should these be allocatable, allocated on first call and saved?.
+
+  constants should have the _wp precision appended (e.g. constants in dstnce) 
+
   minor: #    else at line 446 needs aligning correctly
 
@@ -57 +61 @@
   automatic vs allocatable again. iproc is automatic. iproc is also a bad choice of name (used in lib_mpp and OBS to hold processor number). Perhaps iperproc would be better? blabla.. comments at the top need to be replaced with something meaningful
 
-'''[wiki:TexFiles/Chapters/Chap Chap]_DIA.tex'''
+'''CHAP_DIA.tex'''
 
-  Finally, the documentation is inadequate. Several of the new namelist variables are not described and this is a missed opportunity to expand this section beyond the original, placeholder paragraph.  The relationship between jpnfl and jpnewflo is not explained and the logical of ln_ascii (or rather ln_flo_ascii)  appears to be inverted. One would expect ln_flo_ascii (T) to provide ascii output and (F) to be the new (default) netcdf option. Check comments and use of ln_flo_ascii in documentation and code.
+  Finally, the documentation is inadequate. Several of the new namelist variables are not described and this is a missed opportunity to expand this section beyond the original, placeholder paragraph.  The relationship between jpnfl and jpnewflo is not explained and the logic of ln_ascii (or rather ln_flo_ascii)  appears to be inverted. One would expect ln_flo_ascii (T) to provide ascii output and (F) to be the new (default) netcdf option. Check comments and use of ln_flo_ascii in documentation and code.
 
 === Code Review ===