-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[baggage] Remove unused private field #4318
[baggage] Remove unused private field #4318
Conversation
Signed-off-by: Yuri Shkuro <github@ysh.us>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4318 +/- ##
=======================================
- Coverage 78.9% 78.9% -0.1%
=======================================
Files 254 254
Lines 20650 20646 -4
=======================================
- Hits 16307 16303 -4
Misses 3998 3998
Partials 345 345
|
The following will create a var p Property |
which would still be invalid because of empty key, and validity is the only thing |
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.
I support this change.
We have the hasData
to fail fast if the property key is empty. But all property keys require at least one character. This is used ultimately in NewMember
. This is thus a CPU optimization for the failure case, but adds memory cost for all cases. If we wanted to keep the same error messages, we could do a length check on the key or check if it is empty.
Note: this doesn't apply to Values that can be empty, so we need some way to check if it has been set.
@MrAlias Can you take another look? |
There was no way to create a valid property with
hasData=false
, so the field is not necessary.