Skip to content
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.

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

Close #26: Filter PRs by updated date instead of commit date #1

Merged
merged 1 commit into from
Dec 8, 2022

Conversation

nkrrkn
Copy link
Owner

@nkrrkn nkrrkn commented Dec 8, 2022

This supersedes #205.

This resource can inadvertently miss Pull Requests due to out-of-order commits across PRs. If PR#2 is opened after PR#1, but the head commit of PR#2 is older than the head commit of PR#1, the resource will not include PR#2 in the list of new versions provided to Concourse.

In #205, I removed the date filter entirely. This ensures that the PR resource will find all PRs that match the explicitly-configured filters. While Concourse can detect and ignore duplicate versions, it has to run a database query for every version returned by a check, so removing the date filter entirely would increase load on a Concourse database. (That said, I'm not sure whether this increased load is a particular concern, and other resources don't seem to make much effort to avoid returning duplicate versions from a check.)

To avoid that extra load on a Concourse database, this change instead replaces the filter by commit date in check.go with a filter by updated date in the GraphQL query to list pull requests. This should reduce the number of duplicate versions returned by a check while still allowing the PR resource to detect PRs with out-of-order head commits.

This supersedes #205.

This resource can inadvertently miss Pull Requests due to out-of-order
commits across PRs. If PR#2 is opened after PR#1, but the head commit
of PR#2 is older than the head commit of PR#1, the resource will not
include PR#2 in the list of new versions provided to Concourse.

In #205, I removed the date filter entirely.  This ensures that the PR
resource will find all PRs that match the explicitly-configured filters.
While Concourse can detect and ignore duplicate versions, it has to run
a database query for every version returned by a `check`, so removing
the date filter entirely would increase load on a Concourse database.
(That said, I'm not sure whether this increased load is a particular
concern, and other resources don't seem to make much effort to avoid
returning duplicate versions from a `check`.)

To avoid that extra load on a Concourse database, this change instead
replaces the filter by commit date in `check.go` with a filter by updated
date in the GraphQL query to list pull requests.  This should reduce the
number of duplicate versions returned by a `check` while still allowing
the PR resource to detect PRs with out-of-order head commits.
@nkrrkn nkrrkn merged commit 4e6d302 into nkrrkn:master Dec 8, 2022
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.

2 participants