search

NorESMhub / BLOM

Bergen Layered Ocean Model
GNU Lesser General Public License v3.0
16 stars 25 forks source link

Some cosmetic changes #299

Closed JorgSchwinger closed 8 months ago

JorgSchwinger commented 8 months ago

I went through the code and prettied a few things up a bit (as I believe). If there is something controversial I'm happy to revert it.

jmaerz commented 8 months ago

Hi @JorgSchwinger , ok, seems as if now, I am becoming a bit picky, sorry for that, but: could we think about aligning comments in general with the respective code? - e.g. in carchm , I find it very confusing, where the comment belongs to. etc. - with indenting comments, it becomes clearer, to which code they belong. Further, they visually don't break indentations of code (BTW: this is similarly true and could be done for OMP, while I there understand the reasoning for non-indentation a bit better).

jmaerz commented 8 months ago

Another thing would be the small or capital writing of fortran command, but maybe this goes beyond what we should care about right now. For those, I would prefer to keep MODULE CONTAINS END MODULE in capital, while the rest would be fine also in small. Anyway, this probably is too much for now.

jmaerz commented 8 months ago

Looks like we opened a can of worms :-)

jmaerz commented 8 months ago

Mmh, since this PR provokes some comments, I would suggest to collect them afterward in a small document (or updating older documents on best practices for iHAMOCC coding style).

JorgSchwinger commented 8 months ago

I agree, this seems to be a can of worms...

It looks like we will have to discuss things a bit first. I would suggest that we sit together and update the HAMOCC coding guidelines that we already made some time ago (should be also put into the repository).

For now, I would suggest to leave it here (I have one more pending request though) such that we can proceed making the release v1.4.0 and start working towards v1.5.0.

mvertens commented 8 months ago

@jmaerz - thanks so much for compiling this list. That's super helpful and I agree should be moved to a discussion. I think we need to consider if we want a tool to go through the code and format it as a first pass and then fix things we don't like. It does not have to be emacs. Another point to consider is that most auto-formatters will not handle f77 code - but deal with free format code. At any rate this is a good discussion to have and come to a consensus on.

TomasTorsvik commented 8 months ago

@JorgSchwinger @jmaerz Once this is merged, are we ready for a release-1.4 (i.e. still CMIP6 compatible) from iHAMOCC side?

jmaerz commented 8 months ago

@TomasTorsvik , I think, #300 should also come in, right? - I'll update #289