-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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
cleanup: NewShowWithConfig
-> NewShow
#13600
Conversation
Signed-off-by: George Jenkins <gvjenkins@gmail.com>
pkg/action/show.go
Outdated
|
||
// NewShowWithConfig creates a new Show object with the given configuration. | ||
func NewShowWithConfig(output ShowOutputFormat, cfg *Configuration) *Show { | ||
func NewShow2(output ShowOutputFormat, cfg *Configuration) *Show { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func NewShow2(output ShowOutputFormat, cfg *Configuration) *Show { | |
func NewShow(output ShowOutputFormat, cfg *Configuration) *Show { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR keeps the existing NewShow and renames NewShowWIthConfit to NewShow2. I don't think this was your intent. Is this half done? Can we get the full NewShow -> NewShowWithConfig conversion?
708e83e
to
8468de4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
I missed a commit sorry (During the removal I renamed NewConfig -> NewConfig2 to find callers of that) |
@gjenkins8 please feel free to merge when you're ready. It's approved. |
What this PR does / why we need it:
NewShowWithConfig
replacedNewShow
. RemoveNewShow
, and renameNewShowWithConfig
toNewShow
.Special notes for your reviewer:
If applicable:
docs needed
label should be applied if so)