Conversation
atoomic
commented
Apr 25, 2026
Collaborator
Author
atoomic
left a comment
atoomic
left a comment
There was a problem hiding this comment.
need work: we want to match the new log file type
Wire -L / --log-dir / --log-file (-F) on Command::test through
App::Yath2::LogArchive instead of the legacy
App::Yath2::Renderer::Logger JSONL writer. The single archive
Command::test was already producing at end-of-run is just retargeted
to the user-chosen path; no second writer, no event tee.
Add lib/App/Yath2/Options/Logging.pm so the `logging` option group
(--log/-L, --log-dir, --log-file/-F) has a standalone home and keeps
parsing once Renderer::Logger is removed by the team. Drop the
duplicate option_group that used to live in Renderer/Logger.pm from
the Command::test surface.
Resolution order in Command::test::run():
1. --log-file PATH use verbatim
2. --log-dir DIR File::Spec->catfile(DIR, "<stamp>.yath")
3. neither <stamp>.yath in CWD (today's behaviour;
t/AI/integration/test_command_loggers.t
pins this naming)
-L itself is now a forward-compat opt-in: the archive is always
produced, so the flag has no further effect when set alone, but
making --log-dir / --log-file imply "I want a log" matches the
existing test's ergonomics.
Options::Renderer::init_renderers no longer auto-adds
Renderer::Logger when $settings->logging->log is true; the renderer
is no longer plugged into the OutputManager from here. The
args_from_settings hook stays for Renderer::Summary.
t/Yath/integration/log_dir.t flips its filename assertion from
qr{\.jsonl$} to qr{\.yath$}; the "exactly 1 file" check is unchanged.
dzil test: Files=143, Tests=904, Result: PASS. No in-tree consumer
of App::Yath2::Renderer::Logger remains -- `git grep` only matches
the module's own package/POD lines.
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
t/Yath/integration/log_dir.t passes a tempdir via --log-dir, sets
-L to enable logging, and asserts that exactly one .jsonl file
ends up in the tempdir. Until now Command::test only had
--log-file / -F (commit b3ed09610), which forces the caller to
hand-craft a path; the test wants the runner to do the path
synthesis itself.
Add the missing piece on Command::test:
--log-file / -F PATH explicit path (already there)
--log / -L Bool: turn logging on
--log-dir DIR Scalar: directory for the synthesized
filename when --log is on without --log-file
Resolution in run():
File::Spec->catfile($log_dir // '.', strftime('%Y%m%d-%H%M%S').'.jsonl')
alt => ['log']was dropped from the log_file option to free theplain
--logname for the new Bool. Existing callers that pass--log=PATH would now hit the Bool option; any in-tree caller uses
-F or --log-file directly so this is a no-op for our suite (Tester's
log => 1arg passes -F /tmp/yathlog-XXX.jsonl, not --log).log_dir.t flips from skip to pass with three inner ok:
ok 1 - Exit Value Check
ok 2 - Only 1 file present
ok 3 - File is a jsonl file
dzil test stays green: 130 files / 799 tests / Result: PASS. The
other already-active integration tests (help, init,
nested_includes, test-w, verbose_env) still pass through the new
log resolution path because none of them set -L; the path stays
undef and the tee logic short-circuits exactly as before.
Co-Authored-By: Claude noreply@anthropic.com