Skip to content

Conversation

@huiyifyj
Copy link
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
Copy link
Contributor

@dearchap dearchap left a comment

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

}
var v V
return v.ToString(f.Value)
return f.creator.ToString(f.Value)
Copy link
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
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.

4 participants