From 3661b177661167ddc20b0d8c6bf9516f75e508d3 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Tue, 29 Jun 2021 18:09:42 +0100 Subject: [PATCH 01/11] functionality to have a version that changes on every PR update --- check.go | 6 +++++- in_test.go | 21 +++++++++++++++++++++ models.go | 15 +++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/check.go b/check.go index 64df9e24..347da245 100644 --- a/check.go +++ b/check.go @@ -121,7 +121,11 @@ Loop: continue Loop } } - response = append(response, NewVersion(p)) + newVersionFunc := NewVersion + if request.Source.NewVersionEveryUpdate { + newVersionFunc = NewVersionEveryUpdate + } + response = append(response, newVersionFunc(p)) } // Sort the commits by date diff --git a/in_test.go b/in_test.go index 17e7abfc..6fe5a6ed 100644 --- a/in_test.go +++ b/in_test.go @@ -154,6 +154,26 @@ func TestGet(t *testing.T) { metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, filesString: "README.md\nOther.md\n", }, + { + description: "get works with new version every update", + source: resource.Source{ + Repository: "itsdalmo/test-repository", + AccessToken: "oauthtoken", + NewVersionEveryUpdate: true, + }, + version: resource.Version{ + PR: "pr1", + Commit: "commit1", + CommittedDate: time.Time{}, + ApprovedReviewCount: "0", + State: githubv4.PullRequestStateOpen, + UpdatedAt: &time.Time{}, + }, + parameters: resource.GetParameters{}, + pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), + versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN","updated_at":"0001-01-01T00:00:00Z"}`, + metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, + }, } for _, tc := range tests { @@ -361,6 +381,7 @@ func createTestPR( State: state, ClosedAt: githubv4.DateTime{Time: time.Now()}, MergedAt: githubv4.DateTime{Time: time.Now()}, + UpdatedAt: githubv4.DateTime{Time: time.Now()}, }, Tip: resource.CommitObject{ ID: fmt.Sprintf("commit%s", n), diff --git a/models.go b/models.go index 9e4e7b1c..0c7e600a 100644 --- a/models.go +++ b/models.go @@ -27,6 +27,7 @@ type Source struct { RequiredReviewApprovals int `json:"required_review_approvals"` Labels []string `json:"labels"` States []githubv4.PullRequestState `json:"states"` + NewVersionEveryUpdate bool `json:"new_version_every_update"` } // Validate the source configuration. @@ -76,6 +77,7 @@ type Version struct { CommittedDate time.Time `json:"committed,omitempty"` ApprovedReviewCount string `json:"approved_review_count"` State githubv4.PullRequestState `json:"state"` + UpdatedAt *time.Time `json:"updated_at,omitempty"` } // NewVersion constructs a new Version. @@ -89,6 +91,18 @@ func NewVersion(p *PullRequest) Version { } } +// NewVersionEveryUpdate constructs a new VersionThatChangesOnEveryUpdate. +func NewVersionEveryUpdate(p *PullRequest) Version { + return Version{ + PR: strconv.Itoa(p.Number), + Commit: p.Tip.OID, + CommittedDate: p.UpdatedDate().Time, + ApprovedReviewCount: strconv.Itoa(p.ApprovedReviewCount), + State: p.State, + UpdatedAt: &p.UpdatedAt.Time, + } +} + // PullRequest represents a pull request and includes the tip (commit). type PullRequest struct { PullRequestObject @@ -114,6 +128,7 @@ type PullRequestObject struct { State githubv4.PullRequestState ClosedAt githubv4.DateTime MergedAt githubv4.DateTime + UpdatedAt githubv4.DateTime } // UpdatedDate returns the last time a PR was updated, either by commit From d38d0cbbdeb642907b05f9a526f3358923e956c2 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Tue, 29 Jun 2021 18:15:17 +0100 Subject: [PATCH 02/11] add comment for new constructor --- models.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models.go b/models.go index 0c7e600a..6f0ac7cf 100644 --- a/models.go +++ b/models.go @@ -91,7 +91,9 @@ func NewVersion(p *PullRequest) Version { } } -// NewVersionEveryUpdate constructs a new VersionThatChangesOnEveryUpdate. +// NewVersionEveryUpdate constructs a new Version that changes every time a PR is updated instead of just commits. +// For example, will update if a label is added/removed +// The actions that will cause an update are listed here: https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#webhook-payload-object-33 func NewVersionEveryUpdate(p *PullRequest) Version { return Version{ PR: strconv.Itoa(p.Number), From 776976068f524997ef30a0408ff1e85d5e66cb70 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 10:24:34 +0100 Subject: [PATCH 03/11] refactor and ensure PRs ordered by latest update time instead of commit time if tracking non-commit changes --- README.md | 7 +++++-- check.go | 9 ++++---- check_test.go | 58 +++++++++++++++++++++++++-------------------------- in_test.go | 2 +- models.go | 20 ++++++------------ 5 files changed, 45 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index d46077ff..121f8dd8 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,8 @@ Make sure to check out [#migrating](#migrating) to learn more. | `labels` | No | `["bug", "enhancement"]` | The labels on the PR. The pipeline will only trigger on pull requests having at least one of the specified labels. | | `disable_git_lfs` | No | `true` | Disable Git LFS, skipping an attempt to convert pointers of files tracked into their corresponding objects when checked out into a working copy. | | `states` | No | `["OPEN", "MERGED"]` | The PR states to select (`OPEN`, `MERGED` or `CLOSED`). The pipeline will only trigger on pull requests matching one of the specified states. Default is ["OPEN"]. | +| `track_non_commit_changes` | No | `false` | Will cause a new version of the resource to be created on every pull request update. Changes that do not create new commits, such as adding a label, will trigger the pipeline. + | Notes: - If `v3_endpoint` is set, `v4_endpoint` must also be set (and the other way around). @@ -48,15 +50,16 @@ Notes: #### `check` -Produces new versions for all commits (after the last version) ordered by the committed date. +By default produces new versions for all commits (after the last version) ordered by the committed date. If `track_non_commit_changes` is set to true in the source will produce new versions every time the pull request is updated, orded by the time of the updates. A version is represented as follows: - `pr`: The pull request number. - `commit`: The commit SHA. - `committed`: Timestamp of when the commit was committed. Used to filter subsequent checks. - `approved_review_count`: The number of reviews approving of the PR. +- `updated_at`: Only included if `track_non_commit_changes` is true in the source. Timestamp of when the pull request was last updated. Used to create new versions for non-commit updates and filter subsquent checks. -If several commits are pushed to a given PR at the same time, the last commit will be the new version. +If several commits/updates are pushed to a given PR at the same time, the last commit/update will be the new version. **Note on webhooks:** This resource does not implement any caching, so it should work well with webhooks (should be subscribed to `push` and `pull_request` events). diff --git a/check.go b/check.go index 347da245..30b419cb 100644 --- a/check.go +++ b/check.go @@ -121,11 +121,7 @@ Loop: continue Loop } } - newVersionFunc := NewVersion - if request.Source.NewVersionEveryUpdate { - newVersionFunc = NewVersionEveryUpdate - } - response = append(response, newVersionFunc(p)) + response = append(response, NewVersion(p, request.Source.TrackNonCommitChanges)) } // Sort the commits by date @@ -211,6 +207,9 @@ func (r CheckResponse) Len() int { } func (r CheckResponse) Less(i, j int) bool { + if r[j].UpdatedAt != nil && r[i].UpdatedAt != nil { + return r[j].UpdatedAt.After(*r[i].UpdatedAt) + } return r[j].CommittedDate.After(r[i].CommittedDate) } diff --git a/check_test.go b/check_test.go index 8c422914..0a6e8015 100644 --- a/check_test.go +++ b/check_test.go @@ -45,7 +45,7 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[1]), + resource.NewVersion(testPullRequests[1], false), }, }, @@ -55,11 +55,11 @@ func TestCheck(t *testing.T) { Repository: "itsdalmo/test-repository", AccessToken: "oauthtoken", }, - version: resource.NewVersion(testPullRequests[1]), + version: resource.NewVersion(testPullRequests[1], false), pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[1]), + resource.NewVersion(testPullRequests[1], false), }, }, @@ -69,12 +69,12 @@ func TestCheck(t *testing.T) { Repository: "itsdalmo/test-repository", AccessToken: "oauthtoken", }, - version: resource.NewVersion(testPullRequests[3]), + version: resource.NewVersion(testPullRequests[3], false), pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[2]), - resource.NewVersion(testPullRequests[1]), + resource.NewVersion(testPullRequests[2], false), + resource.NewVersion(testPullRequests[1], false), }, }, @@ -85,7 +85,7 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", Paths: []string{"terraform/*/*.tf", "terraform/*/*/*.tf"}, }, - version: resource.NewVersion(testPullRequests[3]), + version: resource.NewVersion(testPullRequests[3], false), pullRequests: testPullRequests, files: [][]string{ {"README.md", "travis.yml"}, @@ -93,7 +93,7 @@ func TestCheck(t *testing.T) { {"terraform/modules/variables.tf", "travis.yml"}, }, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[2]), + resource.NewVersion(testPullRequests[2], false), }, }, @@ -104,7 +104,7 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", IgnorePaths: []string{"*.md", "*.yml"}, }, - version: resource.NewVersion(testPullRequests[3]), + version: resource.NewVersion(testPullRequests[3], false), pullRequests: testPullRequests, files: [][]string{ {"README.md", "travis.yml"}, @@ -112,7 +112,7 @@ func TestCheck(t *testing.T) { {"terraform/modules/variables.tf", "travis.yml"}, }, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[2]), + resource.NewVersion(testPullRequests[2], false), }, }, @@ -123,10 +123,10 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", DisableCISkip: true, }, - version: resource.NewVersion(testPullRequests[1]), + version: resource.NewVersion(testPullRequests[1], false), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[0]), + resource.NewVersion(testPullRequests[0], false), }, }, @@ -137,10 +137,10 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", IgnoreDrafts: true, }, - version: resource.NewVersion(testPullRequests[3]), + version: resource.NewVersion(testPullRequests[3], false), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[1]), + resource.NewVersion(testPullRequests[1], false), }, }, @@ -151,11 +151,11 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", IgnoreDrafts: false, }, - version: resource.NewVersion(testPullRequests[3]), + version: resource.NewVersion(testPullRequests[3], false), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[2]), - resource.NewVersion(testPullRequests[1]), + resource.NewVersion(testPullRequests[2], false), + resource.NewVersion(testPullRequests[1], false), }, }, @@ -166,12 +166,12 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", DisableForks: true, }, - version: resource.NewVersion(testPullRequests[5]), + version: resource.NewVersion(testPullRequests[5], false), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[3]), - resource.NewVersion(testPullRequests[2]), - resource.NewVersion(testPullRequests[1]), + resource.NewVersion(testPullRequests[3], false), + resource.NewVersion(testPullRequests[2], false), + resource.NewVersion(testPullRequests[1], false), }, }, @@ -186,7 +186,7 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[6]), + resource.NewVersion(testPullRequests[6], false), }, }, @@ -197,10 +197,10 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", RequiredReviewApprovals: 1, }, - version: resource.NewVersion(testPullRequests[8]), + version: resource.NewVersion(testPullRequests[8], false), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[7]), + resource.NewVersion(testPullRequests[7], false), }, }, @@ -215,7 +215,7 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[6]), + resource.NewVersion(testPullRequests[6], false), }, }, @@ -230,7 +230,7 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[9]), + resource.NewVersion(testPullRequests[9], false), }, }, @@ -254,12 +254,12 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", States: []githubv4.PullRequestState{githubv4.PullRequestStateClosed, githubv4.PullRequestStateMerged}, }, - version: resource.NewVersion(testPullRequests[11]), + version: resource.NewVersion(testPullRequests[11], false), pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[9]), - resource.NewVersion(testPullRequests[10]), + resource.NewVersion(testPullRequests[9], false), + resource.NewVersion(testPullRequests[10], false), }, }, } diff --git a/in_test.go b/in_test.go index 6fe5a6ed..b4251b6f 100644 --- a/in_test.go +++ b/in_test.go @@ -159,7 +159,7 @@ func TestGet(t *testing.T) { source: resource.Source{ Repository: "itsdalmo/test-repository", AccessToken: "oauthtoken", - NewVersionEveryUpdate: true, + TrackNonCommitChanges: true, }, version: resource.Version{ PR: "pr1", diff --git a/models.go b/models.go index 6f0ac7cf..61d8afdd 100644 --- a/models.go +++ b/models.go @@ -27,7 +27,7 @@ type Source struct { RequiredReviewApprovals int `json:"required_review_approvals"` Labels []string `json:"labels"` States []githubv4.PullRequestState `json:"states"` - NewVersionEveryUpdate bool `json:"new_version_every_update"` + TrackNonCommitChanges bool `json:"track_non_commit_changes"` } // Validate the source configuration. @@ -81,27 +81,19 @@ type Version struct { } // NewVersion constructs a new Version. -func NewVersion(p *PullRequest) Version { - return Version{ - PR: strconv.Itoa(p.Number), - Commit: p.Tip.OID, - CommittedDate: p.UpdatedDate().Time, - ApprovedReviewCount: strconv.Itoa(p.ApprovedReviewCount), - State: p.State, +func NewVersion(p *PullRequest, trackNonCommitChanges bool) Version { + var updatedAt *time.Time + if trackNonCommitChanges { + updatedAt = &p.UpdatedAt.Time } -} -// NewVersionEveryUpdate constructs a new Version that changes every time a PR is updated instead of just commits. -// For example, will update if a label is added/removed -// The actions that will cause an update are listed here: https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#webhook-payload-object-33 -func NewVersionEveryUpdate(p *PullRequest) Version { return Version{ PR: strconv.Itoa(p.Number), Commit: p.Tip.OID, CommittedDate: p.UpdatedDate().Time, ApprovedReviewCount: strconv.Itoa(p.ApprovedReviewCount), State: p.State, - UpdatedAt: &p.UpdatedAt.Time, + UpdatedAt: updatedAt, } } From 2ea24ce2141ecc17a670289cfff22c47cf21415e Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 10:52:46 +0100 Subject: [PATCH 04/11] fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 121f8dd8..18c5e47c 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ Notes: #### `check` -By default produces new versions for all commits (after the last version) ordered by the committed date. If `track_non_commit_changes` is set to true in the source will produce new versions every time the pull request is updated, orded by the time of the updates. +By default produces new versions for all commits (after the last version) ordered by the committed date. If `track_non_commit_changes` is set to true in the source will produce new versions every time the pull request is updated, ordered by the time of the updates. A version is represented as follows: - `pr`: The pull request number. From 73e8f21d12d13cb855ac95be58b24b697d8d2131 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 11:31:16 +0100 Subject: [PATCH 05/11] remove unneeded | from README --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 18c5e47c..608ec4fa 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,6 @@ Make sure to check out [#migrating](#migrating) to learn more. | `disable_git_lfs` | No | `true` | Disable Git LFS, skipping an attempt to convert pointers of files tracked into their corresponding objects when checked out into a working copy. | | `states` | No | `["OPEN", "MERGED"]` | The PR states to select (`OPEN`, `MERGED` or `CLOSED`). The pipeline will only trigger on pull requests matching one of the specified states. Default is ["OPEN"]. | | `track_non_commit_changes` | No | `false` | Will cause a new version of the resource to be created on every pull request update. Changes that do not create new commits, such as adding a label, will trigger the pipeline. - | Notes: - If `v3_endpoint` is set, `v4_endpoint` must also be set (and the other way around). From e0d8097b7ae1f16b41e6c4f41b470f6bfaeedb03 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 15:28:29 +0100 Subject: [PATCH 06/11] use updated date to exclude old version if track_non_commit_changes set --- check.go | 9 +++++++-- models.go | 6 +++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/check.go b/check.go index 30b419cb..85f6afb6 100644 --- a/check.go +++ b/check.go @@ -26,6 +26,11 @@ func Check(request CheckRequest, manager Github) (CheckResponse, error) { } disableSkipCI := request.Source.DisableCISkip + trackNonCommitChanges := request.Source.TrackNonCommitChanges + savedDate := request.Version.CommittedDate + if trackNonCommitChanges { + savedDate = *request.Version.UpdatedAt + } Loop: for _, p := range pulls { @@ -45,7 +50,7 @@ Loop: } // Filter out commits that are too old. - if !p.UpdatedDate().Time.After(request.Version.CommittedDate) { + if !p.UpdatedDate(trackNonCommitChanges).Time.After(savedDate) { continue } @@ -121,7 +126,7 @@ Loop: continue Loop } } - response = append(response, NewVersion(p, request.Source.TrackNonCommitChanges)) + response = append(response, NewVersion(p, trackNonCommitChanges)) } // Sort the commits by date diff --git a/models.go b/models.go index 61d8afdd..f3feb673 100644 --- a/models.go +++ b/models.go @@ -127,8 +127,12 @@ type PullRequestObject struct { // UpdatedDate returns the last time a PR was updated, either by commit // or being closed/merged. -func (p *PullRequest) UpdatedDate() githubv4.DateTime { +func (p *PullRequest) UpdatedDate(trackNonCommitChanges bool) githubv4.DateTime { date := p.Tip.CommittedDate + if trackNonCommitChanges { + date = p.UpdatedAt + } + switch p.State { case githubv4.PullRequestStateClosed: date = p.ClosedAt From 7040627f7101dbef061795544dd0bb4532bc8699 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 16:31:38 +0100 Subject: [PATCH 07/11] different approach --- check.go | 14 +++-------- check_test.go | 58 ++++++++++++++++++++++---------------------- github.go | 27 +++++++++++---------- in_test.go | 50 ++++++++++++-------------------------- models.go | 30 ++++++++++------------- out_test.go | 66 +++++++++++++++++++++++++-------------------------- 6 files changed, 108 insertions(+), 137 deletions(-) diff --git a/check.go b/check.go index 85f6afb6..50babbbc 100644 --- a/check.go +++ b/check.go @@ -26,11 +26,6 @@ func Check(request CheckRequest, manager Github) (CheckResponse, error) { } disableSkipCI := request.Source.DisableCISkip - trackNonCommitChanges := request.Source.TrackNonCommitChanges - savedDate := request.Version.CommittedDate - if trackNonCommitChanges { - savedDate = *request.Version.UpdatedAt - } Loop: for _, p := range pulls { @@ -50,7 +45,7 @@ Loop: } // Filter out commits that are too old. - if !p.UpdatedDate(trackNonCommitChanges).Time.After(savedDate) { + if !p.ChangeTime().Time.After(request.Version.ChangeTime) { continue } @@ -126,7 +121,7 @@ Loop: continue Loop } } - response = append(response, NewVersion(p, trackNonCommitChanges)) + response = append(response, NewVersion(p)) } // Sort the commits by date @@ -212,10 +207,7 @@ func (r CheckResponse) Len() int { } func (r CheckResponse) Less(i, j int) bool { - if r[j].UpdatedAt != nil && r[i].UpdatedAt != nil { - return r[j].UpdatedAt.After(*r[i].UpdatedAt) - } - return r[j].CommittedDate.After(r[i].CommittedDate) + return r[j].ChangeTime.After(r[i].ChangeTime) } func (r CheckResponse) Swap(i, j int) { diff --git a/check_test.go b/check_test.go index 0a6e8015..8c422914 100644 --- a/check_test.go +++ b/check_test.go @@ -45,7 +45,7 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[1], false), + resource.NewVersion(testPullRequests[1]), }, }, @@ -55,11 +55,11 @@ func TestCheck(t *testing.T) { Repository: "itsdalmo/test-repository", AccessToken: "oauthtoken", }, - version: resource.NewVersion(testPullRequests[1], false), + version: resource.NewVersion(testPullRequests[1]), pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[1], false), + resource.NewVersion(testPullRequests[1]), }, }, @@ -69,12 +69,12 @@ func TestCheck(t *testing.T) { Repository: "itsdalmo/test-repository", AccessToken: "oauthtoken", }, - version: resource.NewVersion(testPullRequests[3], false), + version: resource.NewVersion(testPullRequests[3]), pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[2], false), - resource.NewVersion(testPullRequests[1], false), + resource.NewVersion(testPullRequests[2]), + resource.NewVersion(testPullRequests[1]), }, }, @@ -85,7 +85,7 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", Paths: []string{"terraform/*/*.tf", "terraform/*/*/*.tf"}, }, - version: resource.NewVersion(testPullRequests[3], false), + version: resource.NewVersion(testPullRequests[3]), pullRequests: testPullRequests, files: [][]string{ {"README.md", "travis.yml"}, @@ -93,7 +93,7 @@ func TestCheck(t *testing.T) { {"terraform/modules/variables.tf", "travis.yml"}, }, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[2], false), + resource.NewVersion(testPullRequests[2]), }, }, @@ -104,7 +104,7 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", IgnorePaths: []string{"*.md", "*.yml"}, }, - version: resource.NewVersion(testPullRequests[3], false), + version: resource.NewVersion(testPullRequests[3]), pullRequests: testPullRequests, files: [][]string{ {"README.md", "travis.yml"}, @@ -112,7 +112,7 @@ func TestCheck(t *testing.T) { {"terraform/modules/variables.tf", "travis.yml"}, }, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[2], false), + resource.NewVersion(testPullRequests[2]), }, }, @@ -123,10 +123,10 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", DisableCISkip: true, }, - version: resource.NewVersion(testPullRequests[1], false), + version: resource.NewVersion(testPullRequests[1]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[0], false), + resource.NewVersion(testPullRequests[0]), }, }, @@ -137,10 +137,10 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", IgnoreDrafts: true, }, - version: resource.NewVersion(testPullRequests[3], false), + version: resource.NewVersion(testPullRequests[3]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[1], false), + resource.NewVersion(testPullRequests[1]), }, }, @@ -151,11 +151,11 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", IgnoreDrafts: false, }, - version: resource.NewVersion(testPullRequests[3], false), + version: resource.NewVersion(testPullRequests[3]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[2], false), - resource.NewVersion(testPullRequests[1], false), + resource.NewVersion(testPullRequests[2]), + resource.NewVersion(testPullRequests[1]), }, }, @@ -166,12 +166,12 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", DisableForks: true, }, - version: resource.NewVersion(testPullRequests[5], false), + version: resource.NewVersion(testPullRequests[5]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[3], false), - resource.NewVersion(testPullRequests[2], false), - resource.NewVersion(testPullRequests[1], false), + resource.NewVersion(testPullRequests[3]), + resource.NewVersion(testPullRequests[2]), + resource.NewVersion(testPullRequests[1]), }, }, @@ -186,7 +186,7 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[6], false), + resource.NewVersion(testPullRequests[6]), }, }, @@ -197,10 +197,10 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", RequiredReviewApprovals: 1, }, - version: resource.NewVersion(testPullRequests[8], false), + version: resource.NewVersion(testPullRequests[8]), pullRequests: testPullRequests, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[7], false), + resource.NewVersion(testPullRequests[7]), }, }, @@ -215,7 +215,7 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[6], false), + resource.NewVersion(testPullRequests[6]), }, }, @@ -230,7 +230,7 @@ func TestCheck(t *testing.T) { pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[9], false), + resource.NewVersion(testPullRequests[9]), }, }, @@ -254,12 +254,12 @@ func TestCheck(t *testing.T) { AccessToken: "oauthtoken", States: []githubv4.PullRequestState{githubv4.PullRequestStateClosed, githubv4.PullRequestStateMerged}, }, - version: resource.NewVersion(testPullRequests[11], false), + version: resource.NewVersion(testPullRequests[11]), pullRequests: testPullRequests, files: [][]string{}, expected: resource.CheckResponse{ - resource.NewVersion(testPullRequests[9], false), - resource.NewVersion(testPullRequests[10], false), + resource.NewVersion(testPullRequests[9]), + resource.NewVersion(testPullRequests[10]), }, }, } diff --git a/github.go b/github.go index ab10cbdc..55549d1a 100644 --- a/github.go +++ b/github.go @@ -31,10 +31,11 @@ type Github interface { // GithubClient for handling requests to the Github V3 and V4 APIs. type GithubClient struct { - V3 *github.Client - V4 *githubv4.Client - Repository string - Owner string + V3 *github.Client + V4 *githubv4.Client + Repository string + Owner string + TrackNonCommitChanges bool } // NewGithubClient ... @@ -90,10 +91,11 @@ func NewGithubClient(s *Source) (*GithubClient, error) { } return &GithubClient{ - V3: v3, - V4: v4, - Owner: owner, - Repository: repository, + V3: v3, + V4: v4, + Owner: owner, + Repository: repository, + TrackNonCommitChanges: s.TrackNonCommitChanges, }, nil } @@ -156,10 +158,11 @@ func (m *GithubClient) ListPullRequests(prStates []githubv4.PullRequestState) ([ for _, c := range p.Node.Commits.Edges { response = append(response, &PullRequest{ - PullRequestObject: p.Node.PullRequestObject, - Tip: c.Node.Commit, - ApprovedReviewCount: p.Node.Reviews.TotalCount, - Labels: labels, + PullRequestObject: p.Node.PullRequestObject, + Tip: c.Node.Commit, + ApprovedReviewCount: p.Node.Reviews.TotalCount, + Labels: labels, + TrackNonCommitChanges: m.TrackNonCommitChanges, }) } } diff --git a/in_test.go b/in_test.go index b4251b6f..31c8d205 100644 --- a/in_test.go +++ b/in_test.go @@ -37,13 +37,13 @@ func TestGet(t *testing.T) { version: resource.Version{ PR: "pr1", Commit: "commit1", - CommittedDate: time.Time{}, + ChangeTime: time.Time{}, ApprovedReviewCount: "0", State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{}, pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, + versionString: `{"pr":"pr1","commit":"commit1","change_time":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { @@ -56,13 +56,13 @@ func TestGet(t *testing.T) { version: resource.Version{ PR: "pr1", Commit: "commit1", - CommittedDate: time.Time{}, + ChangeTime: time.Time{}, ApprovedReviewCount: "0", State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{}, pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, + versionString: `{"pr":"pr1","commit":"commit1","change_time":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { @@ -74,7 +74,7 @@ func TestGet(t *testing.T) { version: resource.Version{ PR: "pr1", Commit: "commit1", - CommittedDate: time.Time{}, + ChangeTime: time.Time{}, ApprovedReviewCount: "0", State: githubv4.PullRequestStateOpen, }, @@ -82,7 +82,7 @@ func TestGet(t *testing.T) { IntegrationTool: "rebase", }, pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, + versionString: `{"pr":"pr1","commit":"commit1","change_time":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { @@ -94,7 +94,7 @@ func TestGet(t *testing.T) { version: resource.Version{ PR: "pr1", Commit: "commit1", - CommittedDate: time.Time{}, + ChangeTime: time.Time{}, ApprovedReviewCount: "0", State: githubv4.PullRequestStateOpen, }, @@ -102,7 +102,7 @@ func TestGet(t *testing.T) { IntegrationTool: "checkout", }, pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, + versionString: `{"pr":"pr1","commit":"commit1","change_time":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { @@ -114,7 +114,7 @@ func TestGet(t *testing.T) { version: resource.Version{ PR: "pr1", Commit: "commit1", - CommittedDate: time.Time{}, + ChangeTime: time.Time{}, ApprovedReviewCount: "0", State: githubv4.PullRequestStateOpen, }, @@ -122,7 +122,7 @@ func TestGet(t *testing.T) { GitDepth: 2, }, pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, + versionString: `{"pr":"pr1","commit":"commit1","change_time":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { @@ -134,7 +134,7 @@ func TestGet(t *testing.T) { version: resource.Version{ PR: "pr1", Commit: "commit1", - CommittedDate: time.Time{}, + ChangeTime: time.Time{}, ApprovedReviewCount: "0", State: githubv4.PullRequestStateOpen, }, @@ -150,30 +150,10 @@ func TestGet(t *testing.T) { Path: "Other.md", }, }, - versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, + versionString: `{"pr":"pr1","commit":"commit1","change_time":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN"}`, metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, filesString: "README.md\nOther.md\n", }, - { - description: "get works with new version every update", - source: resource.Source{ - Repository: "itsdalmo/test-repository", - AccessToken: "oauthtoken", - TrackNonCommitChanges: true, - }, - version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, - ApprovedReviewCount: "0", - State: githubv4.PullRequestStateOpen, - UpdatedAt: &time.Time{}, - }, - parameters: resource.GetParameters{}, - pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), - versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0","state":"OPEN","updated_at":"0001-01-01T00:00:00Z"}`, - metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, - }, } for _, tc := range tests { @@ -312,9 +292,9 @@ func TestGetSkipDownload(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.GetParameters{SkipDownload: true}, }, diff --git a/models.go b/models.go index f3feb673..5a04501f 100644 --- a/models.go +++ b/models.go @@ -74,35 +74,29 @@ type MetadataField struct { type Version struct { PR string `json:"pr"` Commit string `json:"commit"` - CommittedDate time.Time `json:"committed,omitempty"` + ChangeTime time.Time `json:"change_time,omitempty"` ApprovedReviewCount string `json:"approved_review_count"` State githubv4.PullRequestState `json:"state"` - UpdatedAt *time.Time `json:"updated_at,omitempty"` } // NewVersion constructs a new Version. -func NewVersion(p *PullRequest, trackNonCommitChanges bool) Version { - var updatedAt *time.Time - if trackNonCommitChanges { - updatedAt = &p.UpdatedAt.Time - } - +func NewVersion(p *PullRequest) Version { return Version{ PR: strconv.Itoa(p.Number), Commit: p.Tip.OID, - CommittedDate: p.UpdatedDate().Time, + ChangeTime: p.ChangeTime().Time, ApprovedReviewCount: strconv.Itoa(p.ApprovedReviewCount), State: p.State, - UpdatedAt: updatedAt, } } // PullRequest represents a pull request and includes the tip (commit). type PullRequest struct { PullRequestObject - Tip CommitObject - ApprovedReviewCount int - Labels []LabelObject + Tip CommitObject + ApprovedReviewCount int + Labels []LabelObject + TrackNonCommitChanges bool } // PullRequestObject represents the GraphQL commit node. @@ -125,11 +119,12 @@ type PullRequestObject struct { UpdatedAt githubv4.DateTime } -// UpdatedDate returns the last time a PR was updated, either by commit -// or being closed/merged. -func (p *PullRequest) UpdatedDate(trackNonCommitChanges bool) githubv4.DateTime { +// ChangeTime returns the last time a PR was updated. +// This time is either by commit/updated_at or being closed/merged. +func (p *PullRequest) ChangeTime() githubv4.DateTime { date := p.Tip.CommittedDate - if trackNonCommitChanges { + + if p.TrackNonCommitChanges { date = p.UpdatedAt } @@ -139,6 +134,7 @@ func (p *PullRequest) UpdatedDate(trackNonCommitChanges bool) githubv4.DateTime case githubv4.PullRequestStateMerged: date = p.MergedAt } + return date } diff --git a/out_test.go b/out_test.go index 4d430c7b..22dc1bbf 100644 --- a/out_test.go +++ b/out_test.go @@ -29,9 +29,9 @@ func TestPut(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{}, pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen), @@ -44,9 +44,9 @@ func TestPut(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Status: "success", @@ -61,9 +61,9 @@ func TestPut(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Status: "failure", @@ -79,9 +79,9 @@ func TestPut(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Status: "failure", @@ -98,9 +98,9 @@ func TestPut(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Status: "failure", @@ -116,9 +116,9 @@ func TestPut(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Status: "failure", @@ -134,9 +134,9 @@ func TestPut(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Comment: "comment", @@ -151,9 +151,9 @@ func TestPut(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ DeletePreviousComments: true, @@ -243,9 +243,9 @@ func TestVariableSubstitution(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Comment: fmt.Sprintf("$%s", variableName), @@ -261,9 +261,9 @@ func TestVariableSubstitution(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Status: "failure", @@ -280,9 +280,9 @@ func TestVariableSubstitution(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - CommittedDate: time.Time{}, + PR: "pr1", + Commit: "commit1", + ChangeTime: time.Time{}, }, parameters: resource.PutParameters{ Comment: "$THIS_IS_NOT_SUBSTITUTED", From 42fb2863105e161b35a536715065bf7bac812d71 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 16:33:39 +0100 Subject: [PATCH 08/11] update README.md --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 608ec4fa..30f2ee95 100644 --- a/README.md +++ b/README.md @@ -54,9 +54,8 @@ A version is represented as follows: - `pr`: The pull request number. - `commit`: The commit SHA. -- `committed`: Timestamp of when the commit was committed. Used to filter subsequent checks. +- `change_time`: Timestamp of when the last change was made. Used to filter subsequent checks. - `approved_review_count`: The number of reviews approving of the PR. -- `updated_at`: Only included if `track_non_commit_changes` is true in the source. Timestamp of when the pull request was last updated. Used to create new versions for non-commit updates and filter subsquent checks. If several commits/updates are pushed to a given PR at the same time, the last commit/update will be the new version. From d94cc1539e932035504c3f8a55b7e6356d87b8cd Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 16:36:30 +0100 Subject: [PATCH 09/11] improve comment --- models.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models.go b/models.go index 5a04501f..9064b83d 100644 --- a/models.go +++ b/models.go @@ -120,7 +120,8 @@ type PullRequestObject struct { } // ChangeTime returns the last time a PR was updated. -// This time is either by commit/updated_at or being closed/merged. +// This time is either by commit time or last update (depending on track_non_commit_changes). +// If the PR is closed/merged it will override the commit/update time. func (p *PullRequest) ChangeTime() githubv4.DateTime { date := p.Tip.CommittedDate From 439fd1278dbb14f0b00f28f6f5e647cf1aca126d Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 17:41:53 +0100 Subject: [PATCH 10/11] edit ChangeTime --- models.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/models.go b/models.go index 9064b83d..b933b844 100644 --- a/models.go +++ b/models.go @@ -120,22 +120,20 @@ type PullRequestObject struct { } // ChangeTime returns the last time a PR was updated. -// This time is either by commit time or last update (depending on track_non_commit_changes). -// If the PR is closed/merged it will override the commit/update time. +// If track_non_commit_changes is true this will always be the updated_at time. +// Otherwise, time is either by commit or close/merge. func (p *PullRequest) ChangeTime() githubv4.DateTime { - date := p.Tip.CommittedDate - if p.TrackNonCommitChanges { - date = p.UpdatedAt + return p.UpdatedAt } + date := p.Tip.CommittedDate switch p.State { case githubv4.PullRequestStateClosed: date = p.ClosedAt case githubv4.PullRequestStateMerged: date = p.MergedAt } - return date } From 378f7d7ffde710736dc42bee7444027f7f1fa1f3 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Wed, 30 Jun 2021 18:03:38 +0100 Subject: [PATCH 11/11] use most recent of commit_date and updated_at --- models.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/models.go b/models.go index b933b844..70ea7b0a 100644 --- a/models.go +++ b/models.go @@ -120,10 +120,13 @@ type PullRequestObject struct { } // ChangeTime returns the last time a PR was updated. -// If track_non_commit_changes is true this will always be the updated_at time. +// If track_non_commit_changes the time is the most recent of committed_date and updated_at. // Otherwise, time is either by commit or close/merge. func (p *PullRequest) ChangeTime() githubv4.DateTime { if p.TrackNonCommitChanges { + if p.Tip.CommittedDate.Time.After(p.UpdatedAt.Time) { + return p.Tip.CommittedDate + } return p.UpdatedAt }