-
Notifications
You must be signed in to change notification settings - Fork 0
ECMWF suggestions for trltog_clean #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ECMWF suggestions for trltog_clean #2
Conversation
67540f7 to
bdea362
Compare
- Lines can be up to 100 characters - Always two continuation characters - One indentation on the next line before the character
bdea362 to
d2dcc54
Compare
|
Sorry, spotted a couple of typos so I've force updated a few commits. |
|
@samhatfield Thank you for this very nice improvements. I will merge your changes, they look OK to me. |
|
I totally agree RE automatic linting. The issue is that there is so much inconsistency in the existing code and any commit to bring the code up to the standards (which are not published anywhere - another thing we need to sort out!) would touch many lines of code and be an "event horizon" in the commit log. I'm much stricter with new code contributions for that reason. Even my commit to "fix" indentation in Perhaps it's possible to have a tool which only lints lines which are changed by a pull request. If you find any Fortran linting tools, let me know. |
|
Thanks for the speedy review BTW! |
Here are some further suggestions to trltog_clean. It looks like a lot in the diff, but if you look at the commits one by one you can see that it's not so significant.
@dhaumont do you need to add a copyright statement for RMI?
Sorry for the nitpicking again, but I find this process of going over every line really useful for understanding what the code does.
BTW @dhaumont, have you discussed these changes with @RyadElKhatibMF? I think he was the original author for these routines so might be interested / concerned about some changes.
Original PR here which will be updated once this one is merged.