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

style: removing unused struct fields #556

Merged
merged 1 commit into from
May 24, 2022
Merged

style: removing unused struct fields #556

merged 1 commit into from
May 24, 2022

Conversation

bobsongplus
Copy link
Contributor

@bobsongplus bobsongplus commented May 9, 2022

/kind cleanup
/kind bug

@netlify
Copy link

netlify bot commented May 9, 2022

Deploy Preview for capsule-documentation canceled.

Name Link
🔨 Latest commit 408633d
🔍 Latest deploy log https://app.netlify.com/sites/capsule-documentation/deploys/62839fd4603b8b00083a6d30

@prometherion
Copy link
Member

Hey @TinySong, thanks for opening your first PR for Capsule!

Could you elaborate a bit more what's the issue you faced? Actually, we got CI and automated tests and expect Capsule to run on Linux-based machines.

return nil
return retryErr
Copy link
Member

Choose a reason for hiding this comment

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

This is definitely a bug, returning the error from the upperscope.

Wondering how this is blocking the build as reported in the PR title.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you reply so quickly. I just recently finding a multi-tenant solution in Kubernetes. then I find capsule, I think capsule is Very accord with my expectations. then Clone the code, and execute make command. output the error
mesage:

~/development/golang/src/capsule/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
golangci-lint run -c .golangci.yml
pkg/webhook/tenant/custom_resource_quota.go:80:5: error is not nil (line 79) but it returns nil (nilerr)
                                return nil
                                ^
controllers/servicelabels/abstract.go:31:2: `scheme` is unused (structcheck)
        scheme *runtime.Scheme
        ^
make: *** [golint] Error 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe I should remove macOS in the title of the PR?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please, although is very weird since we got automation.

Anyway, great finding! 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove on macOS in the git message.

Copy link
Member

Choose a reason for hiding this comment

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

Your PR seems broken and that's right because we should create a sentinel error for the function capsulev1beta1.GetLimitResourceFromTenant.

I already opened #564 with a related fix that has been merged, please rebase over master.

@bobsongplus bobsongplus changed the title fix: build failed from source on macos fix: build failed from source May 10, 2022
@prometherion
Copy link
Member

@TinySong I still see the old commit message, please, could you amend fix: build failed from source on macos in style: removing unused struct fields?

@bobsongplus bobsongplus changed the title fix: build failed from source style: removing unused struct fields May 17, 2022
@bobsongplus
Copy link
Contributor Author

@TinySong I still see the old commit message, please, could you amend fix: build failed from source on macos in style: removing unused struct fields?

@prometherion update git message of PR and rebase the latest code.

@prometherion prometherion added this to the v0.1.2 milestone May 24, 2022
@prometherion prometherion merged commit b9fc508 into projectcapsule:master May 24, 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