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

chore: fix golanglint, checkout prior to setup-go #3206

Merged
merged 2 commits into from
Jan 6, 2024
Merged

Conversation

apeabody
Copy link
Contributor

@apeabody apeabody commented Jan 2, 2024

What this PR does / why we need it:

  • Checkout repo prior to setup-go so go.sum is found and cache works as expected
  • golanglint-ci v1.54 added support for GO 1.21
  • lint fixes for golanglint-ci v1.54
  • convert the inaccessible ErrOldObjectIsNil test to an error specific test

@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2024

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (6503979) 53.70% compared to head (b38f826) 53.71%.
Report is 2 commits behind head on master.

Files Patch % Lines
pkg/pubsub/dapr/fake_dapr_client.go 4.54% 21 Missing ⚠️
pkg/readiness/noop_expectations.go 25.00% 3 Missing ⚠️
pkg/metrics/exporter.go 0.00% 2 Missing ⚠️
cmd/gator/test/test.go 0.00% 1 Missing ⚠️
...controller/externaldata/externaldata_controller.go 0.00% 1 Missing ⚠️
pkg/controller/syncset/syncset_controller.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3206      +/-   ##
==========================================
+ Coverage   53.70%   53.71%   +0.01%     
==========================================
  Files         136      136              
  Lines       12198    12195       -3     
==========================================
  Hits         6551     6551              
+ Misses       5143     5140       -3     
  Partials      504      504              
Flag Coverage Δ
unittests 53.71% <45.28%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@apeabody apeabody changed the title chore: checkout repo prior to setup-go chore: fix golanglint, checkout prior to setup-go Jan 2, 2024
@apeabody apeabody marked this pull request as ready for review January 2, 2024 22:27
@apeabody apeabody requested a review from a team as a code owner January 2, 2024 22:27
Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

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

LGTM

@maxsmythe
Copy link
Contributor

LGTM, though it looks like lint is unhappy?

@apeabody apeabody marked this pull request as draft January 2, 2024 23:55
@apeabody
Copy link
Contributor Author

apeabody commented Jan 2, 2024

LGTM, though it looks like lint is unhappy?

Yeah - Needed to bump the golanglint-ci version for go 1.21 support, still need to add a few more changes for the new lint rules. I'll ping when it's ready.

Signed-off-by: Andrew Peabody <[email protected]>
@@ -21,10 +21,9 @@ func dashExtractor(val string) []string {
b.WriteRune(chr)
prevDash = false
continue
} else {
Copy link
Contributor Author

@apeabody apeabody Jan 3, 2024

Choose a reason for hiding this comment

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

@maxsmythe - Would appreciate an extra set of eyes on this particular change.

Copy link
Member

Choose a reason for hiding this comment

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

Looks right to me.

@apeabody apeabody marked this pull request as ready for review January 3, 2024 00:50
Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM

Copy link
Member

@ritazh ritazh left a comment

Choose a reason for hiding this comment

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

LGTM

@ritazh ritazh merged commit 5325522 into master Jan 6, 2024
16 checks passed
@ritazh ritazh deleted the apeabody-patch-2 branch January 6, 2024 00:45
leewoobin789 pushed a commit to softlee-io/gatekeeper that referenced this pull request Apr 1, 2024
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.

5 participants