Skip to content
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

OSOE-49: Enforce Windows newlines for C# files to avoid false positives with IDE0055 warning. #55

Merged
merged 11 commits into from
May 26, 2022

Conversation

sarahelsaig
Copy link
Member

Copy link
Member

@Piedone Piedone left a comment

Choose a reason for hiding this comment

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

@sarahelsaig
Copy link
Member Author

That wasn't my experience, that's why I have added it back. What do you mean by "current" exactly?

@Piedone
Copy link
Member

Piedone commented May 13, 2022

>=6.0.202, see dotnet/sdk#23972

@sarahelsaig
Copy link
Member Author

That explains it, I had "6.0.102" installed on my Linux desktop. But if we have a specific minimum version requirement within the same major version that really should be documented.

@Piedone
Copy link
Member

Piedone commented May 13, 2022

I'd give us (me) a pass here, since it was a workaround for a .NET SDK bug they introduced with a patch version :D.

@sarahelsaig
Copy link
Member Author

Anyway, we shouldn't remove the fix at least until May 22, because 6.0.200's lifecycle is still active.

@Piedone
Copy link
Member

Piedone commented May 13, 2022

In principle I agree, I'd just really dread to re-add this workaround for these 9 days and have to keep in mind to remove it and retest everything.

@sarahelsaig
Copy link
Member Author

Even after may 22, it's not like everyone will instantly and magically switch over to the latest SDK. Why are you so eager to break backwards compatibility? Does this fix have any serious drawbacks?

@Piedone
Copy link
Member

Piedone commented May 13, 2022

Yeah, but we can still say we don't support older SDKs.

This is a risky workaround because it makes us use the compiler from NuGet instead of the SDK, which is unexpected and discouraged in the package Readme too. It's a hack that we shouldn't keep until absolutely necessary, hence me also jumping on removing it the first time this was possible.

@sarahelsaig
Copy link
Member Author

Yeah, but we can still say we don't support older SDKs.

I suppose we can say that. (I'm not happy about it tho.) Yet the main problem is that we didn't say anything, which confused the person I least want to see confused: myself. :D Instead of deleting the code, I'm going to comment it out and add some text indicating that SDK versions prior to 6.0.102 have to uncomment it.

@Piedone
Copy link
Member

Piedone commented May 13, 2022

I'm sorry :(. Sure! prior to 202, though.

@0liver 0liver merged commit f4408f8 into dev May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants