gavin971 / cvmix

Automatically exported from code.google.com/p/cvmix
0 stars 0 forks source link

Code review request #1

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Branch name: restructure_to_opaque_types

Purpose of code changes on this branch: Initial draft of restructuring of 
cvmix_background to make cvmix_bkgnd_params_type opaque. 

When reviewing my code changes, please focus on: Unsure that all, or any, 
methods are being exercised in regression tests. I also have failed to write 
any protex and non-protex comments. Will correct shortly...

After the review, I'll merge this branch into:
/trunk

Original issue reported on code.google.com by adcr...@gmail.com on 4 Jun 2013 at 6:44

GoogleCodeExporter commented 9 years ago
Code review is done, and branch was merged back onto the trunk. Note that I used

$ svn merge --reintegrate

To handle the update, and it didn't do so well with the log files. For example, 
cvmix_put_get.F90:

log on trunk (last two entries):
$ svn log cvmix_put_get.F90
------------------------------------------------------------------------
r155 | mike.levy.work@gmail.com | 2013-06-07 13:08:49 -0600 (Fri, 07 Jun 2013) 
| 7 lines

Brought branch back onto trunk, use

$ svn merge --reintegrate

After commit, I'll compare log files...

------------------------------------------------------------------------
r114 | mike.levy.work@gmail.com | 2013-05-23 18:12:04 -0600 (Thu, 23 May 2013) 
| 3 lines

This commit can reproduce the tidal mixing coefficients from a column in POP. 
There's still some clean up that needs to be done, and it turns out that POP 
doesn't quite use the vertical deposition function recommended in Simmons et 
al... so I'll need an extra namelist flag to account for that. Also, there is 
no check against "depth_cutoff" yet, even though the variable exists in the 
data structure.

None of the updates Alistair made on the branch (r130, r131, r134, r140, r141) 
are included in this log. I guess the solution is to keep the branch around? 
Maybe I can make it read-only or something.

Original comment by mike.lev...@gmail.com on 7 Jun 2013 at 7:15

GoogleCodeExporter commented 9 years ago
I did some internet sleuthing, and learned that the "-g" flag in "svn log" 
pulls the branch revisions. This is from the trunk:

$ svn log -g cvmix_put_get.F90
------------------------------------------------------------------------
r155 | mike.levy.work@gmail.com | 2013-06-07 13:08:49 -0600 (Fri, 07 Jun 2013) 
| 7 lines

Brought branch back onto trunk, use

$ svn merge --reintegrate

After commit, I'll compare log files...

------------------------------------------------------------------------
r141 | Alistair.Adcroft | 2013-06-05 20:00:20 -0600 (Wed, 05 Jun 2013) | 7 lines
Merged via: r155

Made cvmix_shear_params_type opaque.

o Moved cvmix_shear_params_type into cvmix_shear
o Moved cvmix_put_shear_real function into cvmix_shear
o Added public interfaces for cvmix_put_shear
o Updated driver cmvix_shear_KPP accordingly

Original comment by mike.lev...@gmail.com on 7 Jun 2013 at 7:32