Skip to content

Order versions by push date instead of committer date #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kurtmc merged 1 commit into from
Oct 8, 2026
Merged

Order versions by push date instead of committer date #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 41 additions & 17 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,30 +39,46 @@ Make sure to check out [#migrating](#migrating) to learn more.
| `disable_forks` | No | `true` | Disable triggering of the resource if the pull request's fork repository is different to the configured repository. |
| `ignore_drafts` | No | `false` | Disable triggering of the resource if the pull request is in Draft status. |
| `required_review_approvals` | No | `2` | Disable triggering of the resource if the pull request does not have at least `X` approved review(s). |
| `git_crypt_key` | No | `AEdJVENSWVBUS0VZAAAAA...` | Base64 encoded git-crypt key. Setting this will unlock / decrypt the repository with git-crypt. To get the key simply execute `git-crypt export-key -- - | base64` in an encrypted repository. |
| `git_crypt_key` | No | `AEdJVENSWVBUS0VZAAAAA...` | Base64 encoded git-crypt key. Setting this will unlock / decrypt the repository with git-crypt. To get the key simply execute `git-crypt export-key -- - \| base64` in an encrypted repository. |
| `base_branch` | No | `master` | Name of a branch. The pipeline will only trigger on pull requests against the specified branch. |
| `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"]. |

Notes:
- If `v3_endpoint` is set, `v4_endpoint` must also be set (and the other way around).
- Look at the [Concourse Resources documentation](https://concourse-ci.org/resources.html#resource-webhook-token)

- If `v3_endpoint` is set, `v4_endpoint` must also be set (and the other way around).
- Look at the [Concourse Resources documentation](https://concourse-ci.org/resources.html#resource-webhook-token)
for webhook token configuration.
- When using `required_review_approvals`, you may also want to enable GitHub's branch protection rules to [dismiss stale pull request approvals when new commits are pushed](https://help.github.com/en/articles/enabling-required-reviews-for-pull-requests).
- When using `required_review_approvals`, you may also want to enable GitHub's branch protection rules to [dismiss stale pull request approvals when new commits are pushed](https://help.github.com/en/articles/enabling-required-reviews-for-pull-requests).

## Behaviour

#### `check`
### `check`

Produces new versions for all commits (after the last version) ordered by the committed date.
Produces a new version for the tip of every pull request pushed since the last version, ordered by when it was pushed.
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.
- `committed`: Timestamp of when the commit was committed.
- `changed`: Timestamp the version is ordered by, and used to filter subsequent checks: when the commit was pushed to
the pull request's branch, taken from the [repository activity API](https://docs.github.com/en/rest/repos/repos#list-repository-activities),
but never earlier than the pull request was opened, closed or merged.
- `state`: The pull request state.

Ordering by the push rather than the committed date matters because Concourse only schedules `version: every` inputs
ranked above the newest version a job has already built: a rebased commit keeps an old committer date, so ordering by
it would rank the commit below versions that were already built and the push would never be scheduled. GitHub's
GraphQL API no longer populates `Commit.pushedDate`, hence the activity API.

If several commits are pushed to a given PR at the same time, the last commit will be the new version.
Only open pull requests updated since the last version are looked up (one REST call each); a failed lookup fails the
check so that no push is skipped past. Pull requests from forks have no activity in the base repository, and a push
GitHub has not recorded cannot be found, so those are ordered by their committed date instead.

A version that only names a pull request, as in `fly check-resource --from pr:123`, produces just that pull request's
current tip, ordered as the newest version regardless of its dates.

**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).
Expand All @@ -71,7 +87,7 @@ generate notifications over the webhook. So if you have a repository with little
you'll need to discover those versions with `check_every: 1m` for instance. `check` in this resource is not a costly operation,
so normally you should not have to worry about the rate limit.

#### `get`
### `get`

| Parameter | Required | Example | Description |
|----------------------|----------|----------|------------------------------------------------------------------------------------|
Expand All @@ -87,13 +103,14 @@ into master. This ensures that we are both testing and setting status on the exa
input. Because the base of the PR is not locked to a specific commit in versions emitted from `check`, a fresh
`get` will always use the latest commit in master and *report the SHA of said commit in the metadata*. Both the
requested version and the metadata emitted by `get` are available to your tasks as JSON:

- `.git/resource/version.json`
- `.git/resource/metadata.json`
- `.git/resource/changed_files` (if enabled by `list_changed_files`)

The information in `metadata.json` is also available as individual files in the `.git/resource` directory, e.g. the `base_sha`
is available as `.git/resource/base_sha`. For a complete list of available (individual) metadata files, please check the code
[here](https://github.com/telia-oss/github-pr-resource/blob/master/in.go#L66).
is available as `.git/resource/base_sha`. For a complete list of available (individual) metadata files, please check
[the metadata written by `in.go`](https://github.com/telia-oss/github-pr-resource/blob/master/in.go#L66).

When specifying `skip_download` the pull request volume mounted to subsequent tasks will be empty, which is a problem
when you set e.g. the pending status before running the actual tests. The workaround for this is to use an alias for
Expand All @@ -116,8 +133,7 @@ the base, Concourse will reuse the volume (i.e. not trigger a new `get`) if it s
unexpected results (#5). As such, re-testing a PR against a newer version of the base is best done by *pushing an
empty commit to the PR*.


#### `put`
### `put`

| Parameter | Required | Example | Description |
|----------------------------|----------|--------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------|
Expand Down Expand Up @@ -197,25 +213,31 @@ translates to 5000 requests, whereas for the V4 API (GraphQL) the calculation i
https://developer.github.com/v4/guides/resource-limitations/#calculating-a-rate-limit-score-before-running-the-call

Ref the above, here are some examples of running `check` against large repositories and the cost of doing so:

- [concourse/concourse](https://github.com/concourse/concourse): 51 open pull requests at the time of testing. Cost 2.
- [torvalds/linux](https://github.com/torvalds/linux): 305 open pull requests. Cost 8.
- [kubernetes/kubernetes](https://github.com/kubernetes/kubernetes): 1072 open pull requests. Cost: 22.

`check` additionally spends one V3 API call per pull request updated since the last version, to find when its tip was pushed.

For the other two operations the costing is a bit easier:

- `get`: Fixed cost of 1. Fetches the pull request at the given commit.
- `put`: Uses the V3 API and has a min cost of 1, +1 for each of `status`, `comment` and `comment_file` etc.

## Migrating

If you are coming from [jtarchie/github-pullrequest-resource][original-resource], its important to know that this resource is inspired by *but not a drop-in replacement for* the original. Here are some important differences:

#### New parameters:
### New parameters

- `source`:
- `v4_endpoint` (see description above)
- `put`:
- `comment` (see description above)

#### Parameters that have been renamed:
### Parameters that have been renamed

- `source`:
- `repo` -> `repository`
- `ci_skip` -> `disable_ci_skip` (the logic has been inverted and its `true` by default)
Expand All @@ -228,7 +250,8 @@ If you are coming from [jtarchie/github-pullrequest-resource][original-resource]
- `put`:
- `comment` -> `comment_file` (because we added `comment`)

#### Parameters that are no longer needed:
### Parameters that are no longer needed

- `src`:
- `uri`: We fetch the URI directly from the Github API instead.
- `private_key`: We clone over HTTPS using the access token for authentication.
Expand All @@ -238,7 +261,8 @@ If you are coming from [jtarchie/github-pullrequest-resource][original-resource]
- `get`:
- `fetch_merge`: We are opinionated and always do a fetch_merge.

#### Parameters that did not make it:
### Parameters that did not make it

- `src`:
- `authorship_restriction`
- `label`
Expand Down Expand Up @@ -291,7 +315,7 @@ query {
}
```

### Clone/Fetch

#### Clone/Fetch
Clone/fetch is defaulted to a `git_depth` : `1`
This will double up to a `MaxDepth` if we can't find a common ancestor
115 changes: 89 additions & 26 deletions check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ package resource

import (
"fmt"
"github.com/shurcooL/githubv4"
"log"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"time"

"github.com/shurcooL/githubv4"
)

// Check (business logic)
Expand All @@ -25,6 +28,14 @@ func Check(request CheckRequest, manager Github) (CheckResponse, error) {
return nil, fmt.Errorf("failed to get last commits: %w", err)
}

// A version that only names a pull request (fly check-resource --from pr:N) asks for that
// pull request's tip alone; it is not a version to order against or to return.
if request.Version.IsPullRequestHint() {
pulls = pullRequestNumbered(pulls, request.Version.PR)
request.Version = Version{}
}
previousPR, _ := strconv.Atoi(request.Version.PR)

disableSkipCI := request.Source.DisableCISkip

Loop:
Expand All @@ -44,33 +55,17 @@ Loop:
continue
}

versionTime := p.Age()
if len(request.Source.StatusFilters) > 0 {
for _, statusFilter := range request.Source.StatusFilters {
// "zero time - epoch = 0"
versionTime = time.Time{}
isValid := false
for _, prStatus := range p.Tip.Status.Contexts {
if prStatus.Context == statusFilter.Context && strings.EqualFold(prStatus.State, statusFilter.State) {
// requires the given status exists and that it matches the desired state
isValid = true
// set the versionTime to the latest time of all
// the status checks

if prStatus.CreatedAt.After(versionTime) {
versionTime = prStatus.CreatedAt.Time
}
break
}
}
if !isValid {
continue Loop
}
}
versionTime, statusOK := statusVersionTime(p, request.Source.StatusFilters)
if !statusOK {
continue
}

// Filter out commits that are too old.
if !p.Age().After(request.Version.ChangedDate) {
// Skip pull requests with no activity since the last version, and the one that produced
// it, before spending an API call on them.
if !p.UpdatedAt.After(request.Version.ChangedDate) {
continue
}
if p.Number == previousPR && p.Tip.OID == request.Version.Commit && p.State == request.Version.State && len(request.Source.StatusFilters) == 0 {
continue
}

Expand Down Expand Up @@ -108,6 +103,20 @@ Loop:
continue
}

pushed, err := pushedDate(manager, p)
if err != nil {
return nil, err
}
changed := p.ChangedDate(pushed)

// Filter out commits that are too old.
if !changed.After(request.Version.ChangedDate) {
continue
}
if len(request.Source.StatusFilters) == 0 {
versionTime = changed
}

// Fetch files once if paths/ignore_paths are specified.
var files []string

Expand Down Expand Up @@ -163,6 +172,60 @@ Loop:
return response, nil
}

func pullRequestNumbered(pulls []*PullRequest, number string) []*PullRequest {
for _, p := range pulls {
if strconv.Itoa(p.Number) == number {
return []*PullRequest{p}
}
}
return nil
}

// statusVersionTime is the time of the newest status check the filters require, and whether the
// pull request's tip has every one of them.
func statusVersionTime(p *PullRequest, filters []StatusFilter) (time.Time, bool) {
var versionTime time.Time
for _, statusFilter := range filters {
// "zero time - epoch = 0"
versionTime = time.Time{}
isValid := false
for _, prStatus := range p.Tip.Status.Contexts {
if prStatus.Context == statusFilter.Context && strings.EqualFold(prStatus.State, statusFilter.State) {
// requires the given status exists and that it matches the desired state
isValid = true
// set the versionTime to the latest time of all
// the status checks

if prStatus.CreatedAt.After(versionTime) {
versionTime = prStatus.CreatedAt.Time
}
break
}
}
if !isValid {
return time.Time{}, false
}
}
return versionTime, true
}

// pushedDate is when the pull request's tip was pushed, or zero when the order does not depend on
// it: a fork's branch has no activity in this repository, a merged pull request is ordered by its
// merge, and a push GitHub has not recorded is ordered by its committer date.
func pushedDate(manager Github, p *PullRequest) (time.Time, error) {
if p.IsCrossRepository || p.State == githubv4.PullRequestStateMerged {
return time.Time{}, nil
}
pushed, found, err := manager.PushedDate(p.HeadRefName, p.Tip.OID)
if err != nil {
return time.Time{}, fmt.Errorf("pr %d: failed to look up when %s was pushed: %w", p.Number, p.Tip.OID, err)
}
if !found {
log.Printf("pr %d: no push of %s recorded on %s, ordering it by its committer date", p.Number, p.Tip.OID, p.HeadRefName)
}
return pushed, nil
}

// ContainsSkipCI returns true if a string contains [ci skip] or [skip ci].
func ContainsSkipCI(s string) bool {
re := regexp.MustCompile("(?i)\\[(ci skip|skip ci)\\]")
Expand Down
Loading
Loading