Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[uwtools_integration] Integrate ics/lbcs #266
base: uwtools_integration
Are you sure you want to change the base?
[uwtools_integration] Integrate ics/lbcs #266
Changes from 35 commits
6bdab4c
46a3227
1af2517
b7884cc
b188301
4753089
93ac362
e437b3d
bee31be
e661d1c
7360ec1
ed58157
8dde45f
9f16342
d6c3786
4eff96a
3b65a6c
7116141
c310cb8
9a0ab65
226d335
8a3ed18
8b7dd87
2f709e3
b83c5af
d9701bf
fc59204
12fb677
f485be8
fccb757
7bc7732
80bca46
4300a3e
91e15dd
7d81efb
25fd2b4
1460cff
0e2f6e0
093fbf0
affa5d4
d74b410
86db971
d9488d9
fb398c0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this will work -- whether
disable-next
will apply to the next block and not just the next line, for those suppressors that relate to this function. If not, this line should probably move to the top of the module (after the#!
) because, without-next
, it applies to every line of code until the end of the module, and I think we should make this kind of interference with the linter more visibly obvious.But some of this is also not bad advice from the linter: This function in 177 lines long. Could it be refactored into smaller, purpose-specific functions for better readability? I see what look like section comments describing various (maybe) unrelated operations, each of which could be its own function, which would then be somewhat self-documenting, e.g. we could change
to function call
and factor out the code to that function. If nothing else, there is a giant
if
/else
block where the two alternatives might better be encapsulated in two functions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaving this open for now to see what you think of the current shorter version. It might be a cleaner version as well if I moved the file linking or the loop off to a helper function?