Impl HasCooldownPassed#4
Conversation
…1.14.8 > testdata/terraform-tags-v1.14.8.json
a7b6fd8 to
7374159
Compare
7374159 to
5da139d
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new client helper to determine whether a specific GitHub release tag is “old enough” relative to a configured cooldown period, likely to support workflows that need to gate actions until a release has aged past a threshold.
Changes:
- Add
Client.HasCooldownPassed(...)which fetches a release by tag and comparespublished_atagainst the cooldown duration. - Add unit tests for the new method using
httpmock. - Add a new JSON fixture for the GitHub “get release by tag” API response.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
client.go |
Adds HasCooldownPassed method using GetReleaseByTag and cooldown comparison logic. |
client_test.go |
Adds table-driven tests for HasCooldownPassed using a new fixture. |
testdata/terraform-tags-v1.14.8.json |
Adds fixture JSON for GET /releases/tags/{tag} response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ts := release.GetPublishedAt() | ||
|
|
||
| if ts.IsZero() { | ||
| return false, nil | ||
| } |
There was a problem hiding this comment.
HasCooldownPassed considers only published_at, but unlike GetLatestTagName it does not exclude draft/prerelease releases. This can return true for a prerelease tag even though the rest of the client treats prereleases as ineligible. Consider returning false when release.GetDraft() or release.GetPrerelease() is true to keep behavior consistent.
| httpmock.RegisterResponder( | ||
| "GET", | ||
| "https://api.github.com/repos/hashicorp/terraform/releases/tags/v1.14.8", | ||
| httpmock.NewStringResponder(200, readTestData("testdata/terraform-tags-v1.14.8.json")), | ||
| ) | ||
|
|
||
| type args struct { |
There was a problem hiding this comment.
HasCooldownPassed supports GitHub Enterprise via ClientParams.BaseURL, but the new test only mocks api.github.com and never exercises the baseURL path (even though args.baseURL exists). Add a test case similar to TestClient_GetLatestTagName that sets baseURL and registers the corresponding responder URL.
| ts := release.GetPublishedAt() | ||
|
|
||
| if ts.IsZero() { | ||
| return false, nil | ||
| } |
There was a problem hiding this comment.
The ts.IsZero() branch in HasCooldownPassed (when published_at is missing/zero) is new behavior but isn’t covered by tests. Consider adding a fixture/response with a null/empty published_at (or a draft release) to assert the method returns false without error.
No description provided.