-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Do not allow GetType and compare ignoring case #9972
Do not allow GetType and compare ignoring case #9972
Conversation
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.
It is however functional change which can break someone.
Please consider to put it under change wave.
Thank you @rokonec for the review, the possible breaking change was discussed in the issue. msbuild/src/Build/Evaluation/Expander.cs Lines 5302 to 5311 in c820d6b
What do you think? FYI @baronfel |
@f-alizada in this case it is not a big deal. However, if I understand it correctly, we try to make it easier for customer by putting ALL functional changes into changewave and using escapehatches for long term (infinite) support of some optional features. in this particular case it make sense to me that it will be under both opt-in escape hatch and changewave. That being said, customer could have been already broken with former case sensitive change, so maybe in this case we can leave it without changewave. What is your take on it @rainersigwald |
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.
As an extension to an already-documented breaking change with an existing documented escape hatch I'm comfortable not putting this behind a changewave (for once!).
Fixes #9967
Context
The GetType method is still available if called like "gettype"
Changes Made
Compare the method name ignoring the case
Testing
Added two tests