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

nogo: fix unconvert exclude rules #130993

Merged

Conversation

srosenberg
Copy link
Member

In [1], a regression was added in exclude_files
for the unconvert linter. This regression only
affects builds instrumented with coverage; see [2] for further details.

As a workaround for [2], we extend exclude_files to exclude Go sources generated from protobuf,
having the optional suffix of the form _[0-9]+.

[1] #124155
[2] bazel-contrib/rules_go#4110

Epic: none

Release note: None

@srosenberg srosenberg requested a review from a team as a code owner September 19, 2024 03:39
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@srosenberg srosenberg requested review from a team, nameisbhaskar and vidit-bhat and removed request for a team September 19, 2024 03:39
In [1], a regression was added in `exclude_files`
for the `unconvert` linter. This regression only
affects builds instrumented with coverage; see [2]
for further details.

As a workaround for [2], we extend `exclude_files`
to exclude Go sources generated from protobuf,
having the optional suffix of the form `_[0-9]+`.

[1] cockroachdb#124155
[2] bazel-contrib/rules_go#4110

Epic: none

Release note: None
@srosenberg srosenberg force-pushed the sr/nogo_fix_unconvert_exclusion branch from 24205ee to 70d0f8c Compare September 19, 2024 04:04
@srosenberg
Copy link
Member Author

srosenberg commented Sep 19, 2024

The following now builds,

./dev build short --cross=linuxarm -- --collect_code_coverage --bazel_code_coverage

Copy link
Contributor

@nameisbhaskar nameisbhaskar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@srosenberg
Copy link
Member Author

TFTR!

bors r=vidit-bhat,nameisbhaskar

@craig craig bot merged commit 163ee9a into cockroachdb:master Oct 25, 2024
23 checks passed
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.

4 participants