Skip to content

Bump viper and revert go-difflib and go-spew #5832

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
Closed
wants to merge 1 commit into from
Closed

Bump viper and revert go-difflib and go-spew #5832

wants to merge 1 commit into from

Conversation

skitt
Copy link
Member

@skitt skitt commented Jan 6, 2025

The viper bump brings a simplified dependency structure and allows the go-difflib and go-spew bumps to be reverted (to their last respective releases).

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 6, 2025
@skitt skitt mentioned this pull request Jan 6, 2025
Merged
@skitt
Copy link
Member Author

skitt commented Jan 6, 2025

/hold wait for the actual viper release

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 6, 2025
@koba1t
Copy link
Member

koba1t commented Jan 7, 2025

/assign

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 7, 2025
@koba1t
Copy link
Member

koba1t commented Jan 7, 2025

Hi @skitt
Thanks for helping to clean up dependencies!

Can you remove two below exclude directives in this PR?

kustomize/cmd/gorepomod/go.mod

Lines 5 to 15 in 2867f35

exclude (
// These dependencies were updated to "master" in spf13/viper, but have no
// code-changes since the last release. While a fix was merged in viper,
// it's not released yet, and it may take time before other (indirect)
// dependencies also downgraded.
//
// Exclude these versions so that go modules picks their latest release
// before that. For additional details, see; https://github.com/kubernetes-sigs/kustomize/pull/5830
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
)

kustomize/hack/go.mod

Lines 5 to 15 in 2867f35

exclude (
// These dependencies were updated to "master" in spf13/viper, but have no
// code-changes since the last release. While a fix was merged in viper,
// it's not released yet, and it may take time before other (indirect)
// dependencies also downgraded.
//
// Exclude these versions so that go modules picks their latest release
// before that. For additional details, see; https://github.com/kubernetes-sigs/kustomize/pull/5830
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
)

@skitt
Bump viper and revert go-difflib and go-spew
5a22e89 
The viper bump brings a simplified dependency structure and allows the
go-difflib and go-spew bumps to be reverted (to their last respective
releases).

Signed-off-by: Stephen Kitt <skitt@redhat.com>
@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 8, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: skitt
Once this PR has been reviewed and has the lgtm label, please ask for approval from koba1t. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@skitt
Copy link
Member Author

skitt commented Jan 8, 2025

Can you remove two below exclude directives in this PR?

Done!

@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 8, 2025
stormqueen1990
stormqueen1990 reviewed Jan 15, 2025
View reviewed changes
Copy link
Member

@stormqueen1990 stormqueen1990 left a comment

Choose a reason for hiding this comment

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

/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 15, 2025
@stormqueen1990
Copy link
Member

/test all

@koba1t
Copy link
Member

koba1t commented Mar 15, 2025

Hi @skitt

Do you feel this PR still need?
If that is no, I'll close!

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2025
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

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-sigs/prow repository.

@skitt
Copy link
Member Author

skitt commented Mar 17, 2025

Huh, I forgot about this and redid it as #5873!

@skitt skitt closed this Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncapps ncapps Awaiting requested review from ncapps

1 more reviewer

@stormqueen1990 stormqueen1990 stormqueen1990 left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@stormqueen1990 stormqueen1990

@koba1t koba1t

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@skitt @koba1t @k8s-ci-robot @stormqueen1990