Skip to content

Use self properties instead of declared new variables - #2143

Closed
huiyifyj wants to merge 1 commit into
urfave:mainfrom
huiyifyj:reduce-var-declare
Closed

huiyifyj wants to merge 1 commit into
urfave:mainfrom
huiyifyj:reduce-var-declare

Conversation

@huiyifyj

Copy link
Copy Markdown
Member

What type of PR is this?

  • cleanup

What this PR does / why we need it:

Reduce declared variables and use self properties directly in TakesValue and GetDefaultText methods.

Reduce declared variables and use self properties directly in `TakesValue` and `GetDefaultText` methods.
@huiyifyj
huiyifyj requested a review from a team as a code owner May 21, 2025 01:54

@dearchap dearchap left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While this change is well intentioned I dont think we have sufficient tests to verify

Comment thread flag_impl.go
}
var v V
return v.ToString(f.Value)
return f.creator.ToString(f.Value)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is f.creator hasnt been initialized yet ? Define a flag(with empty DefaultText) and call GetDefaultTtext without doing a cmd.Run and see what happens. It is likely the code will crash

@meatballhat meatballhat added kind/cleanup describes internal cleanup / maintaince status/waiting-for-response Waiting for response from original requester area/v3 relates to / is being considered for v3 labels Jun 14, 2025
@meatballhat meatballhat changed the title optimize: use self properties instead of declared new variables Use self properties instead of declared new variables Jun 14, 2025
@coilysiren

Copy link
Copy Markdown
Member

I'm closing everything whose last update was in 2025 or earlier, to reduce maintainer burden while we coordinate a roll-over of who is doing the primary work day to day. You are highly encouraged to re-open this if it's still live for you!

@coilysiren coilysiren closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/v3 relates to / is being considered for v3 kind/cleanup describes internal cleanup / maintaince status/waiting-for-response Waiting for response from original requester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants