Skip to content

Conversation

@ksylvan
Copy link
Collaborator

@ksylvan ksylvan commented Jan 4, 2026

Fix the last_pr_sync setting during PR incoming processing

Summary

This PR fixes the last_pr_sync timestamp logic in the changelog generator to use the version date instead of the current time when creating a new changelog entry.

Files Changed

File Change Type Description
cmd/generate_changelog/internal/changelog/processing.go Modified Updated the timestamp used for last_pr_sync cache value

Code Changes

processing.go

The key change modifies how the last_pr_sync cache value is set after creating a new changelog entry:

Before:

// Update last_pr_sync to current time
if err := g.cache.SetLastPRSync(time.Now()); err != nil {

After:

// Update last_pr_sync to the version date (not current time)
// This ensures future runs will fetch PRs merged after this version
if err := g.cache.SetLastPRSync(versionDate); err != nil {

Reason for Changes

The previous implementation used time.Now() to set the last_pr_sync timestamp, which is incorrect behavior. This could cause issues in scenarios where:

  1. There's a delay between when PRs are merged and when the changelog is generated
  2. The version being processed has a date in the past (e.g., processing historical versions)
  3. PRs merged between the version date and the current time would be incorrectly excluded from future changelog runs

By using versionDate instead, the sync timestamp accurately reflects the cutoff point for the version being processed, ensuring that subsequent runs will correctly fetch PRs merged after that version's date.

Impact of Changes

  • Correctness: Future changelog generation runs will now correctly identify PRs that should be included, based on when versions were actually released rather than when the changelog tool was run.
  • Consistency: The sync state will be deterministic based on version dates rather than execution time.
  • Backward Compatibility: This is a behavioral fix; the function signature and cache interface remain unchanged.

Potential Bugs / Considerations

  1. Assumption on versionDate: This change assumes versionDate is always defined and valid at this point in the code. If versionDate could be zero-valued or nil, this could set an incorrect sync timestamp.

  2. Timezone handling: Ensure that versionDate and any future PR timestamp comparisons use consistent timezone handling.

Test Plan

  1. Run the changelog generator for a past version and verify that last_pr_sync is set to the version date, not the current time
  2. Create a new changelog entry, then verify subsequent runs correctly fetch PRs merged after the version date
  3. Verify edge cases where version dates are in the past or future relative to current time

Additional Notes

The updated comments clearly explain the reasoning behind using versionDate, which improves code maintainability for future contributors.

- Change last_pr_sync to use versionDate instead of time.Now()
- Ensure future runs fetch PRs merged after the version date
- Add clarifying comments explaining the sync timing logic
@ksylvan ksylvan self-assigned this Jan 4, 2026
@ksylvan ksylvan merged commit a5ac60c into danielmiessler:main Jan 4, 2026
1 check passed
@ksylvan ksylvan deleted the kayvan/one-more-ci-cd-changelog-fix branch January 4, 2026 07:30
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