Skip to content
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

Document and update method '_emit_log()' in all Test modules #1310

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

tgreenx
Copy link
Contributor

@tgreenx tgreenx commented Nov 29, 2023

Purpose

This PR documents and updates the recently added _emit_log() internal method. See the Changes section below.

Context

#1302

Changes

  • Move '_emit_log()' to the proper section in each Test modules
  • Add documentation for '_emit_log()' in each Test modules
  • Align implementation of 'Zonemaster::Engine::Test::Basic::_emit_log()' with its counterpart from other Test modules
  • Make 'Zonemaster::Engine::Test::Address::find_private_address()' an internal method

How to test this PR

Tests should pass.

POD documentation should show correctly, e.g. using pod2text.

@tgreenx tgreenx added the A-Documentation Area: Documentation only. label Nov 29, 2023
@tgreenx tgreenx added this to the v2023.2 milestone Nov 29, 2023
- Move '_emit_log()' to the proper section in each Test modules
- Add documentation for '_emit_log()' in each Test modules
- Align implementation of 'Zonemaster::Engine::Test::Basic::_emit_log()' with its counterpart from other Test modules
- Make 'Zonemaster::Engine::Test::Address::find_private_address()' an internal method
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM, just a tiny suggestion. But this could work without it.


Takes a string (message tag) and a reference to a hash (arguments).

Returns a L<Zonemaster::Engine::Logger::Entry> object.
Copy link

Choose a reason for hiding this comment

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

Suggestion: specify that the entry is defined with the Address testcase value or something like that so that it is clear that this internal method is different in each module.

@tgreenx tgreenx merged commit f2a2db2 into zonemaster:develop Dec 4, 2023
3 checks passed
@tgreenx tgreenx deleted the update-emit_log branch December 4, 2023 10:53
@hannaeko hannaeko self-assigned this Jan 10, 2024
@hannaeko hannaeko added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Documentation Area: Documentation only. S-ReleaseTested Status: The PR has been successfully tested in release testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants