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

Minor wording adjustment to migration guide #845

Merged
merged 2 commits into from
Nov 28, 2023
Merged

Conversation

stevenaw
Copy link
Member

I've noticed a bit of hesitation on social media about the changes to assert styles. I thought a minor wording change could help here, mostly around dropping the word "convert" in one of the headings to minimize apparent friction.

I'm unsure which of these could be preferable so I've gone with the shorter, first alternative in this PR for now:

- #### Convert Classic Assert into NUnit 4.x equivalent
+ #### Using Classic Asserts in NUnit 4.x

or

- #### Convert Classic Assert into NUnit 4.x equivalent
+ #### Using Classic-style Asserts in NUnit 4.x

@manfred-brands @SeanKilleen I know you each put a lot of work into the guide, so I'd appreciate your thoughts on this

@@ -96,7 +96,7 @@ Assert.That(actualText, Does.StartWith("42"), $"Expected '{actualText}' to start
There are no code fixers for `FileAssert` and `DirectoryAssert`. They could be added, but we don't expect these to be
used too much.

#### Convert Classic Assert into NUnit 4.x equivalent
#### Using Classic Asserts in NUnit 4.x
Copy link
Member

Choose a reason for hiding this comment

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

The steps outline how to migrate existing code to the new naming.
If you don't like Convert, as a minimum it should be Update.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea, I like "Update" over "Convert" as it feels "lighter".

What are your thoughts on Classic Asserts vs Classic-style Asserts ? I'm trying to think of wording for this section which emphasizes to the reader that they can choose to stick with their chosen style of asserts if it is the classic style - even if it means they must update the referenced class.

Copy link
Member

Choose a reason for hiding this comment

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

What are your thoughts on Classic Asserts vs Classic-style Asserts ?

I'm happy with either.

Copy link
Member

Choose a reason for hiding this comment

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

I suggest Updating from Classic Asserts in NUnit 4.x

And I suggest it should be "Classic Asserts". We're calling the whole idea of them classic, not just the style.

@OsirisTerje
Copy link
Member

@stevenaw Can you link to some of these social media posts? If there are many negative on this, we should rethink, or invite to a poll, to figure out what the overall response is.

@SeanKilleen
Copy link
Member

@OsirisTerje nobody likes to have their cheese moved. I mentioned in prior conversation that it'll be one of those things that seems more like a psychic weight than it will actually end up being. We made the decision, and we've provided multiple bridges to the new world; no need to rethink based on social media (though it is still good to see it)

@stevenaw
Copy link
Member Author

stevenaw commented Nov 28, 2023

Agreed with @SeanKilleen re: moving of cheese

@OsirisTerje
I wouldn't say "negative" so much as just a single comment on a twitter feed about the change, but it had reminded me of @rprouse 's comment in the v4 planning issue about the friction of removing ExpectedExceptionAttribute in the move from v2 -> v3 and the desire he expressed at that point about making for an easy transition with whatever changes we included in v4. My knee-jerk reaction was then to think about softening language for that part of the docs.

No need in my mind to conduct outreach or change approach at this time either. I agree we've provided a clear migration path and considered several different scenarios in it for users.

@stevenaw
Copy link
Member Author

Thank you @SeanKilleen and @manfred-brands for your feedback. I've updated the section title to Updating from Classic Asserts in NUnit 4.x

Copy link
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

Thanks @stevenaw

@SeanKilleen SeanKilleen merged commit f656542 into master Nov 28, 2023
7 checks passed
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.

4 participants