Sample PR - Issue2219 remove obsolete attributes #2338
Conversation
|
I'm in favour of merging this into 3.8. I appreciate it doesn't follow SemVer - but we don't follow SemVer too strictly anyway. We should just come to peace with that, and call what we do NUnitVer instead. 😄 They've been obsolete since 3.0, and people have had plenty of time to refactor. What I would like to see is everything that was obsoleted in 3.0 removed in one go, to minimise user disruption. As far as I can see, the only other thing is the |
| @@ -38,13 +38,13 @@ public class LiveTest | |||
| private PairCounter pairsTested = new PairCounter(); | |||
|
|
|||
| [OneTimeSetUp] | |||
| public void TestFixtureSetUp() | |||
| public void OneTimeSetUp() | |||
There was a problem hiding this comment.
Unneeded but OK. We often refer to the OneTimeSetUp of a fixture this way in the docs.
| [TestCase(typeof(Class3))] | ||
| [TestCase(typeof(Class4))] | ||
| [TestCase(typeof(TestSetupClass))] | ||
| [TestCase(typeof(TestSearDownClass))] |
|
Looks good to me as well if you fix the misspelled class name. |
|
I am good with this too. As @CharliePoole says, the spelling needs fixing, but after that, I think we should merge. I like that this only changes things that are easy to update with a global search and replace. I would prefer to deal with changes that will require hand updating as a SemVer breaking change. |
|
@stevenaw if you want to resolve conflicts and fix that one spelling error, I think we can merge this and take the rest of the obsolete code in steps. |
….com/stevenaw/nunit into issue2219-remove-obsolete-attributes
|
Thanks @rprouse @CharliePoole @ChrisMaddock . Build completes locally, but running the tests locally has 2 failures due to |
|
Looks good - re-running CI. 🙂 |
|
CI passed, merging |
A sample PR for Issue #2219 to visualize the impact of removing obsolete attributes.