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.

Sign up for GitHub

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

Jump to bottom

Replace github/pkg/errors with sigs.k8s.io/kustomize/kyaml/errors #4970

Merged
merged 1 commit into from Jan 13, 2023
Merged

Replace github/pkg/errors with sigs.k8s.io/kustomize/kyaml/errors #4970

merged 1 commit into from Jan 13, 2023

Conversation

cailynse
Copy link
Contributor

Addresses: #4964

Replace github/pkg/errors with sigs.k8s.io/kustomize/kyaml/errors

errors.Wrap -> errors.WrapPrefixf
errors.Wrapf -> errors.WrapPrefixf
errors.New -> errors.Errorf
errors.Cause -> functions updated to only check errors.Is (use case was only to check if an error is a given type)
errors.WithStack -> removed because it was not impacting output (will investigate outside of this PR)

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 10, 2023
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 10, 2023
cailynse
cailynse commented Jan 10, 2023
View reviewed changes
plugin/builtin/sortordertransformer/SortOrderTransformer.go
@@ -72,7 +72,7 @@ func (p *plugin) Transform(m resmap.ResMap) (err error) {
p.applyDefaults()
err = p.validate()
if err != nil {
return errors.WithStack(err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could not get stack output using this locally. It was recently introduced and not release so removing the with stack functionality for now.

@cailynse cailynse marked this pull request as ready for review January 10, 2023 21:03
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 10, 2023
KnVerey
KnVerey reviewed Jan 10, 2023
View reviewed changes
api/Makefile Outdated Show resolved Hide resolved
api/internal/builtins/HelmChartInflationGenerator.go Outdated
@@ -252,7 +252,7 @@ func (p *HelmChartInflationGeneratorPlugin) Generate() (rm resmap.ResMap, err er
// try to remove the contents before first "---" because
// helm may produce messages to stdout before it
stdoutStr := string(stdout)
if idx := strings.Index(stdoutStr, "---"); idx != -1 {
if idx := strings.Index(stdoutStr, "\n---\n"); idx != -1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

accidental change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one I am not sure about. It's a generated file - might I have changed something elsewhere causing this?

Copy link
Contributor

@KnVerey KnVerey Jan 13, 2023 •
edited

Choose a reason for hiding this comment

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

Yes, this one is generated from plugin/builtin/helmchart.... The change is gone for that file, so I think you need to re-run make generate-kustomize-builtin-plugins. It's not good that CI didn't catch the need to do that! Filed #4977

api/internal/plugins/loader/loader.go Outdated Show resolved Hide resolved
api/internal/plugins/loader/loader.go Outdated Show resolved Hide resolved
api/internal/target/errmissingkustomization.go Show resolved Hide resolved
cmd/depprobcheck/go.mod Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 11, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 11, 2023
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 11, 2023
@koba1t
Copy link
Member

koba1t commented Jan 11, 2023

Hi @cailynse.

I found remain lines in generated go files like here.
Could you try make clean?

@k8s-ci-robot
Copy link
Contributor

@cailynse: This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@cailynse cailynse force-pushed the remove-gh-errors-pkg-4964 branch 2 times, most recently from beac9b9 to d908b27 Compare January 11, 2023 19:31
@cailynse
Copy link
Contributor Author

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 11, 2023
KnVerey
KnVerey reviewed Jan 13, 2023
View reviewed changes
api/go.mod
@@ -28,6 +27,7 @@ require (
github.com/kr/pretty v0.2.1 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
github.com/pkg/errors v0.9.1 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

This is coming from github.com/evanphx/json-patch, which k/k itself also depends on and has not been updated to remove the errors dependency: https://github.com/kubernetes/kubernetes/blob/4802d7bb62c2623be8e4f940f6b5c1fcddd6c744/go.mod#L34

@KnVerey
Copy link
Contributor

KnVerey commented Jan 13, 2023

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 13, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cailynse, KnVerey

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 13, 2023
@k8s-ci-robot k8s-ci-robot merged commit 8ab4b8d into kubernetes-sigs:master Jan 13, 2023
@KnVerey KnVerey mentioned this pull request Jan 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KnVerey KnVerey KnVerey left review comments

@mmorel-35 mmorel-35 mmorel-35 left review comments

@mortent mortent Awaiting requested review from mortent

@natasha41575 natasha41575 Awaiting requested review from natasha41575

Assignees

@KnVerey KnVerey

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@cailynse @k8s-ci-robot @koba1t @KnVerey @mmorel-35