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

Use specific constraint #839

Merged
merged 2 commits into from
Feb 10, 2025
Merged

Use specific constraint #839

merged 2 commits into from
Feb 10, 2025

Conversation

manfred-brands
Copy link
Member

Fixes #824
Fixes #826
Fixes #828

@mikkelbu The first commit is not directly related but was necessary to allow NUnit 4 specific names to be only tested when building against NUnit instead of making up the names in the analyzer code.

The second commit add the new analyzer and code fix.

There was a small change in the documentation tests as they were not testing for the new Style analyzers.

If you think this analyzer should be in a different category, that is fine by me.

Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

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

Looks great @manfred-brands and I think it is fine to use the 4XX category for this (at least I don't have any strong opinion about this).

I've added a couple of suggestions, but that is all

documentation/NUnit4002.md Outdated Show resolved Hide resolved
documentation/NUnit4002.md Outdated Show resolved Hide resolved
@mikkelbu
Copy link
Member

mikkelbu commented Feb 2, 2025

I forgot a question or two - mostly just curious, if I overlooked something.

It there a reason for NUnitFrameworkConstants and NUnitLegacyFrameworkConstants being abstract and NUnitV4FrameworkConstants is not?

Also is there a difference between abstract and static in this case - i.e. the classes only containing public const string... properties and no inheritance (NUnitFrameworkConstants was static before, but it was changed to abstract).

@manfred-brands
Copy link
Member Author

I forgot a question or two - mostly just curious, if I overlooked something.

It there a reason for NUnitFrameworkConstants and NUnitLegacyFrameworkConstants being abstract and NUnitV4FrameworkConstants is not?

Also is there a difference between abstract and static in this case - i.e. the classes only containing public const string... properties and no inheritance (NUnitFrameworkConstants was static before, but it was changed to abstract).

I have no access to a PC until next week, but from what I remember there was an error at one time that I could not use a static class somewhere, although I don't think I used inheritance. It might no longer be the case. I'll check when I get back.

The main difference between static and abstract here would be inheritance.

documentation/NUnit4002.md Outdated Show resolved Hide resolved
manfred-brands and others added 2 commits February 10, 2025 11:24
NUnitFrameworkConstants
NUnitLegacyFrameworkConstants
NUnitV4FrameworkConstants

Main reason was for V4 as this can only be tested when compiling against V4
This is for:
Is.EqualTo(false) => Is.False
Is.EqualTo(true) => Is.True
Is.EqualTo(null) => Is.Null
Is.EqualTo(default) => Is.Default

Apply suggestions from code review

Co-authored-by: Mikkel Nylander Bundgaard <[email protected]>
@manfred-brands manfred-brands merged commit ac3a370 into master Feb 10, 2025
6 checks passed
@manfred-brands manfred-brands deleted the UseSpecificConstraint branch February 10, 2025 03:51
@mikkelbu mikkelbu added this to the Release 4.7 milestone Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants