-
Notifications
You must be signed in to change notification settings - Fork 42
Implement TypeValueFormatter (#48) #78
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
Conversation
Pull Request Test Coverage Report for Build 14128706657Details
💛 - Coveralls |
90d53ca to
09e24b5
Compare
7613b23 to
c74233e
Compare
ITaluone
left a comment
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.
Great work!
Just a few comments for you to consider..
Tests/FluentAssertions.Specs/Collections/CollectionAssertionSpecs.AllBeAssignableTo.cs
Show resolved
Hide resolved
Tests/FluentAssertions.Specs/Types/TypeAssertionSpecs.BeAssignableTo.cs
Outdated
Show resolved
Hide resolved
Tests/FluentAssertions.Specs/Types/TypeAssertionSpecs.HaveAccessModifier.cs
Show resolved
Hide resolved
Tests/FluentAssertions.Specs/Types/TypeAssertionSpecs.HaveAccessModifier.cs
Show resolved
Hide resolved
IT-VBFK
left a comment
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.
LGTM
The most failures look way cleaner and way more readable than before.
Maybe the suggestions from @msmolka can be a refinement in a follow-up PR...
I've been busy lately. Then we have the variants
|
Co-authored-by: ITaluone <[email protected]>
Co-authored-by: ITaluone <[email protected]>
fc8f0e8 to
5ec660d
Compare
|
|
@IT-VBFK I reverted some changes were previously there was |
IT-VBFK
left a comment
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.
Quite cool :)
|
Thanks, I agree with the changes, looks good |
|
Just a quick question to make sure I'm not missing anything. Is this, low-key, a breaking change that will force people to remove If so, I think it would be a good idea to add a feature flag to disable it, like this: <!-- End develop adds to opt in for old behavior -->
<ItemGroup>
<RuntimeHostConfigurationOption Include="AwesomeAssertions.TypeValueFormatter" Value="false" />
</ItemGroup>Then, in the AA code, should check: AppContext.TryGetSwitch("AwesomeAssertions.TypeValueFormatter", out var enabled);Whether this should be opt-in or opt-out is another question. Just a side note: MSFT has switched to AA and is using v8, so we want to avoid causing any issues. |
No change in user code is required. The change affects only the error message if one uses In previous versions changes of error messages haven't been seen as being breaking. |



Fixes #48
IMPORTANT
./build.sh --target spellcheckor.\build.ps1 --target spellcheckbefore pushing and check the good outcome