Clean up *.build.json files #79
Conversation
WalkthroughThe changes refactor the way log files are cleaned up and how build information is handled. The Changes
Sequence Diagram(s)sequenceDiagram
participant PL as Process_Logs
participant PT as Prettify
participant FS as File System
PL->>PT: clean_logs(log_file)
PT->>FS: Iterate over suffixes and delete files
FS-->>PT: Deletion results
PT-->>PL: Return status (1)
sequenceDiagram
participant SB as Scoreboard
participant PT as Prettify
participant FS as File System
SB->>SB: list_logs (scanning directory with $timestamp_re)
SB->>SB: get_latest returns (timestamp, build text)
SB->>PT: delete_prettify_output(log_prefix)
PT->>FS: Delete files with defined suffixes
FS-->>PT: Acknowledge deletion
PT-->>SB: Return deletion status
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
common/prettify.pm (1)
35-43: Good addition of a centralized cleanup function.The new
delete_prettify_outputfunction centralizes the file deletion logic for prettified output files, making the code more maintainable. When file types change, only this function needs to be updated instead of multiple places in the codebase.Consider adding error handling to catch and optionally log any issues that might occur during the deletion process:
sub delete_prettify_output { my $prefix = shift(); for my $suffix (".txt", "_JUnit.xml", "_Full.html", "_Brief.html", "_Totals.html", "_Config.html", ".build.json") { - unlink($prefix . $suffix); + my $file = $prefix . $suffix; + unlink($file) or warn "Could not delete $file: $!" if -e $file; } }scoreboard.pl (1)
114-127: Improved defensive programming in list_logs function.The updated function now checks if the directory handle was successfully created before proceeding, preventing potential issues with undefined handles.
Consider returning a more descriptive error message that includes the reason for failure:
if (!defined($dh)) { - print STDERR ("ERROR: Could not read $dir\n"); + print STDERR ("ERROR: Could not read $dir: $!\n"); return 0; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
command/process_logs.pm(1 hunks)common/prettify.pm(1 hunks)scoreboard.pl(9 hunks)
🔇 Additional comments (8)
command/process_logs.pm (1)
199-200: Good refactoring to use centralized deletion function.The code now delegates file deletion to the
delete_prettify_outputfunction from the Prettify module, reducing code duplication and making maintenance easier.scoreboard.pl (7)
107-107: Good addition of a reusable timestamp regex.Defining a single timestamp regex pattern improves consistency and maintainability across the codebase.
140-141: Good refactoring to use centralized deletion function.The code now delegates file deletion to the
Prettify::delete_prettify_outputfunction instead of directly callingunlink, improving code maintainability.
149-165: Improved get_latest function to return more useful information.The function now returns both the timestamp and the text of the latest build, making it more useful for callers.
750-751: Updated code to use the new get_latest signature.The call to
get_latestnow correctly captures both return values: the timestamp and the text.
753-753: Improved condition logic with more precise comparisons.The condition now correctly uses the timestamp value instead of relying on just the latest build information existence.
767-772: Consolidated build update logic.The code now directly checks if the latest text is defined before setting the latest build information, rather than using a separate function. This simplifies the code and makes it more straightforward.
839-841: Good refactoring to use centralized deletion function.Similar to other instances, this code now uses the centralized
Prettify::delete_prettify_outputfunction for file deletion.
jwillemsen
left a comment
jwillemsen
left a comment
There was a problem hiding this comment.
Not reviewed the code, thanks for looking at this
Revert changes from DOCGroup#79 that broke OpenDDS scoreboard.
3 participants
Fixes #78
Summary by CodeRabbit
New Features
Refactor