issues: filter by exact title match when looking for existing issue #96389
Conversation
renatolabs
requested review from
srosenberg and
smg260
and removed request for
a team
|
This change is |
renatolabs
force-pushed
the
rc/fix-existing-gh-issue
branch
from
a5d0424 to
eb35304
Compare
There was a problem hiding this comment.
Reviewed 20 of 20 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @renatolabs, @smg260, and @srosenberg)
pkg/cmd/internal/issues/issues_test.go line 180 at r1 (raw file):
factories := make([]issueFactory, 0, len(suffixes)) for k, suffix := range suffixes { suffix := suffix // capture range variable
shouldn't k be captured as well?
There was a problem hiding this comment.
Nice fix, I'm sure this will prevent unwarranted confusion in the future.
Reviewable status:
This changes the logic used to find existing and related issues when posting test failures on GitHub. Previously, we would rely on the GitHub API to find issues with the a certain title; however, the GitHub API does not provide a way to filter by exact title match. Instead, issues that have a "similar" name are returned. As a consequence, when a test fails, we might comment on an existing issue for a different test, which is confusing and may mask failures. As a means of example, check issue cockroachdb#91594: failures for `schemachange/mixed-versions` and `schemachange/mixed-versions-compat` are both posted there for this reason. This commit does some further filtering on our end to ensure that an issue is only considered existing or related if the titles match exactly. Epic: none Release note: None
renatolabs
force-pushed
the
rc/fix-existing-gh-issue
branch
from
eb35304 to
4a517ff
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/internal/issues/issues_test.go line 180 at r1 (raw file):
Previously, herkolategan (Herko Lategan) wrote…
shouldn't
kbe captured as well?
Hah, nice catch, it's funny how often I still make this mistake even after thinking about it for so long while writing that linter. Fixed.
There was a problem hiding this comment.
Reviewable status:
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/internal/issues/issues_test.go line 180 at r1 (raw file):
Previously, renatolabs (Renato Costa) wrote…
Hah, nice catch, it's funny how often I still make this mistake even after thinking about it for so long while writing that linter. Fixed.
Easy to miss those! And still something I'm getting used to, having worked in Java for a long time, I'm very use to the compiler complaining that a variable needs to be final if you're capturing it in a lambda.
|
bors r=herkolategan TFTR! |
|
Build succeeded: |
This changes the logic used to find existing and related issues when posting test failures on GitHub. Previously, we would rely on the GitHub API to find issues with the a certain title; however, the GitHub API does not provide a way to filter by exact title match. Instead, issues that have a "similar" name are returned. As a consequence, when a test fails, we might comment on an existing issue for a different test, which is confusing and may mask failures.
As a means of example, check issue #91594: failures for
schemachange/mixed-versionsandschemachange/mixed-versions-compatare both posted there for this reason.This commit does some further filtering on our end to ensure that an issue is only considered existing or related if the titles match exactly.
Epic: none
Release note: None