Skip to content

Conversation

@dcarp
Copy link
Collaborator

@dcarp dcarp commented Oct 10, 2025

No description provided.

@dcarp dcarp requested a review from Copilot October 10, 2025 22:26
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the output of the "Tag a Release" GitHub workflow by making the tag age information more human-readable and adding clearer logging messages. The changes enhance the user experience when monitoring the workflow's decision-making process.

  • Enhanced human-readable time formatting for tag age display
  • Added clearer logging messages to indicate workflow decisions
  • Improved script robustness with better error handling and configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@dcarp dcarp force-pushed the polish_tag_a_release_workflow branch from 304b80d to baa5368 Compare October 10, 2025 22:35
@dcarp dcarp requested a review from Copilot October 10, 2025 22:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@dcarp dcarp force-pushed the polish_tag_a_release_workflow branch from baa5368 to 9f0576c Compare October 10, 2025 22:39
@dcarp dcarp changed the title chore: polish 'Tag a Release' ouput chore: use human readable durations in the output of 'Tag a Release' workflow Oct 10, 2025
@dcarp dcarp requested a review from Copilot October 10, 2025 22:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +40 to +54
human_readable_duration() {
local secs=$1
local days=$((secs/86400))
local hours=$((secs%86400/3600))
local mins=$((secs%3600/60))
if ((days > 0)); then
printf '%dd %dh %dm' "$days" "$hours" "$mins"
elif ((hours > 0)); then
printf '%dh %dm' "$hours" "$mins"
elif ((mins > 0)); then
printf '%dm' "$mins"
else
printf '%ds' "$((secs%60))"
fi
}
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

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

The function doesn't handle negative durations. If TAG_TIME is somehow in the future compared to NOW, this could result in negative values being passed to the function, leading to incorrect output.

Copilot uses AI. Check for mistakes.
Comment on lines +55 to +56
# 2 weeks minus 12 hours to ensure reliable 2 weeks timing of the daily cron job
MAX_AGE=$((14 * 24 * 3600 - 12 * 3600))
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

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

[nitpick] The magic number calculation for MAX_AGE could be made more readable by using intermediate variables or adding a comment explaining the specific values (14 days = 1209600 seconds, 12 hours = 43200 seconds).

Suggested change
# 2 weeks minus 12 hours to ensure reliable 2 weeks timing of the daily cron job
MAX_AGE=$((14 * 24 * 3600 - 12 * 3600))
# Calculate MAX_AGE as 2 weeks (14 days) minus 12 hours, in seconds.
TWO_WEEKS_SECS=$((14 * 24 * 3600)) # 14 days = 1,209,600 seconds
TWELVE_HOURS_SECS=$((12 * 3600)) # 12 hours = 43,200 seconds
MAX_AGE=$((TWO_WEEKS_SECS - TWELVE_HOURS_SECS)) # 1,166,400 seconds

Copilot uses AI. Check for mistakes.
@dcarp dcarp merged commit e00d112 into bazel-contrib:main Oct 10, 2025
11 checks passed
@dcarp dcarp deleted the polish_tag_a_release_workflow branch October 10, 2025 22:58
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.

1 participant