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

Use new build image 0.20.3 #5904

Merged
merged 3 commits into from
Apr 22, 2022
Merged

Use new build image 0.20.3 #5904

merged 3 commits into from
Apr 22, 2022

Conversation

chaudum
Copy link
Contributor

@chaudum chaudum commented Apr 13, 2022

What this PR does / why we need it:

The new build image uses the latest version v1.45.2 of golangci-lint. With this version running the make lint locally yields the same results as running on CI.

Special notes for your reviewer:

  • Requires Other: trigger build of loki-build-image:0.20.3 #5924 so that 0.20.3 is built
  • This PR contains commits from Update golangci-lint in build image #5901
  • The changes in file .drone/drone.yml were generated by make drone
  • Code changes are fixes for the linter:
     pkg/storage/stores/tsdb/index/index.go:1525:61: unexported-return: exported func ReadFingerprintOffsetsTable returns unexported type index.fingerprintOffsets, which can be annoying to use (revive)
     func ReadFingerprintOffsetsTable(bs ByteSlice, off uint64) (fingerprintOffsets, error) {
                                                              ^
     pkg/ingester/ingester.go:554:59: unexported-return: exported method GetOrCreateInstance returns unexported type *ingester.instance, which can be annoying to use (revive)
     func (i *Ingester) GetOrCreateInstance(instanceID string) *instance {
    

@chaudum chaudum requested a review from a team as a code owner April 13, 2022 12:39
@chaudum chaudum force-pushed the chaudum/fix-make-lint branch from 29bbde6 to f0f972e Compare April 13, 2022 12:53
@chaudum chaudum requested a review from jeschkies April 14, 2022 09:53
Copy link
Contributor

@jeschkies jeschkies left a comment

Choose a reason for hiding this comment

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

Ok, according to the build **logs** of you build image change. It pushed grafana/loki-build-image:0.20.2

And I know why. You didn't regenerate the drone.yml 🙈 make check-drone-drift was supposed to catch that.

.drone/drone.yml Outdated Show resolved Hide resolved
@@ -27,7 +27,7 @@ DOCKER_IMAGE_DIRS := $(patsubst %/Dockerfile,%,$(DOCKERFILES))
BUILD_IN_CONTAINER ?= true

# ensure you run `make drone` after changing this
BUILD_IMAGE_VERSION := 0.20.1
BUILD_IMAGE_VERSION := 0.20.3
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you recent change just overrode 0.20.2 it should be that

Suggested change
BUILD_IMAGE_VERSION := 0.20.3
BUILD_IMAGE_VERSION := 0.20.2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chaudum chaudum force-pushed the chaudum/fix-make-lint branch from f0f972e to b13e6b1 Compare April 20, 2022 06:31
@chaudum chaudum requested a review from jeschkies April 20, 2022 06:38
@@ -551,7 +551,7 @@ func (i *Ingester) Push(ctx context.Context, req *logproto.PushRequest) (*logpro
return &logproto.PushResponse{}, err
}

func (i *Ingester) GetOrCreateInstance(instanceID string) *instance {
func (i *Ingester) GetOrCreateInstance(instanceID string) *instance { //nolint:revive
Copy link
Contributor

Choose a reason for hiding this comment

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

This change seems unrelated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new linter version reports: unexported-return: exported method GetOrCreateInstance returns unexported type *ingester.instance

@chaudum chaudum requested a review from jeschkies April 22, 2022 06:50
@jeschkies jeschkies merged commit b6ce382 into main Apr 22, 2022
@jeschkies jeschkies deleted the chaudum/fix-make-lint branch April 22, 2022 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants