-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[release/6.0] Fix: add missed open curly #61332
Conversation
Tagging subscribers to this area: @buyaa-n |
@buyaa-n what is the impact on the customer though? can you add to customer impact section: how commonly do folks do this, how many apps are broken, is there a workaround, etc. Or is this just a bug someone happened to spot but with low impact. |
I would not expect it to be used much and would break many apps, correct me if I'm wrong @romkatv, how is this is impacting you? For me, this is low impact bug and I requested this for servicing only because it is a regression and the fix has no risk
Sure, i will update that section |
Thanks. Just being a regression isn't reason enough on its own to service shipped product. We look for evidence of sufficient impact, as well as risk. |
I maintain a code generator that, among other things, invokes |
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.
Thanks.
Thanks, @romkatv, I have updated the customer impact section accordingly |
Backport of #61320 to release/6.0
Fixes #61282
/cc @buyaa-n @annchous
Customer Impact
A customer reported a regression in
CustomAttributeTypedArgument.ToString()
introduced in 6.0, the customer using it for a code generator where it was producing a valid C# code, which was broken by this bug, there is a workaround by implementing custom ToString() methods or updating the string produced, etc. but it is better to be fixed in the SDK. There could be more customers affected by this, we might get more bug reports after they port to the .Net 6.0 release. The fix is trivial, just adding the missing curly brace back to the string, nothing new added.Testing
A new unit test was added that reproduces the bug and confirms the fix with the expected string produced by CustomAttributeTypedArgument.ToString()
Risk
Minimal, the only risk I could think of is if the customer did a workaround that updates the string result from CustomAttributeTypedArgument.ToString() that would be broken by this fix, but as this is fixed in 7.0 they will be broken again with 7.0
CC @jeffhandley @danmoseley