Skip to content

Add reference to log_at_level API#160

Merged
Steve Mullerworth (stevemullerworth) merged 4 commits intoMetOffice:mainfrom
MatthewHambley:LogAtLevel
Oct 8, 2025
Merged

Add reference to log_at_level API#160
Steve Mullerworth (stevemullerworth) merged 4 commits intoMetOffice:mainfrom
MatthewHambley:LogAtLevel

Conversation

@MatthewHambley
Copy link
Copy Markdown
Collaborator

@MatthewHambley Matthew Hambley (MatthewHambley) commented Oct 8, 2025

Description

In order to support a common usage pattern around only calculating debug values if they are actually going to be logged, an addition has been made to the logging API. This change documents that.

Closes #134

Linked issues

The API change is implemented in ticket 4682 of the core repository.

Type of change

  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • This change requires a documentation update.

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

  • New tests have been added.
  • Tests have been modified to accommodate this change.
  • GitHub workflows have been changed or added.

Checklist:

  • The change has been checked for design compliance by an experienced SSE
  • My code follows the style guidelines of this project.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes, for both debug and optimised builds.
  • No AI tools have been used in the creation of this change.

Reasoning on why any of the above boxes have not been checked

This is a documentation only change.

Review Checks (To be filled in by the reviewer/s)

  • Has the developer completed the appropriate sections above?
  • Is the change compliant with LFRic Core principles?
  • Is the testing coverage sufficient?
  • Have any technical debt workarounds identified had issues opened and interested parties notified?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor comments.

Once initialised :ref:`logging` may proceed as normal.

When you have logged the last thing you care to log call ``final_logger``. The
string passed to it is used only to output a final message. This rather
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest removing the "only", as the underlying function does do other hidden work (shutting down the communicator, for example). For the same reason the "This rather..." sentence should be removed as it may imply that it is not necessary to call final_logger (it may not be necessary, but we would like people to do it).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewritten

call log_event(log_scratch_space, log_level_debug)
end if

Note that there is duplication of ``log_level_debug`` here, if they are not
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good to note the duplication, but I could imagine that a block could include log_level_trace messages and call subroutines that have log_level_info messages.

I wouldn't labour these points though as they are moderately obvious. I'd suggest "Note that it would be normal to duplicate log_level_debug in the final call to log_event".

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewritten. I want to labour it a little to make people stop and think about what they would expect.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All fine now.

@stevemullerworth Steve Mullerworth (stevemullerworth) merged commit ced2933 into MetOffice:main Oct 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document log driver

2 participants