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

Change names of "OrEqualTo" assertion methods #1673

Merged
merged 1 commit into from Sep 6, 2021

Conversation

gliljas
Copy link
Contributor

@gliljas gliljas commented Aug 27, 2021

The names of the GreaterOrEqualsTo methods (and similar) is a bit off, but more importantly they don't follow the established terminology.

@gliljas
Copy link
Contributor Author

gliljas commented Aug 27, 2021

Should I rebase?

@dennisdoomen
Copy link
Member

dennisdoomen commented Aug 27, 2021

For people upgrading to 6.x, even an obsolete attribute might be a build error. So, I'm in doubt whether we should do that right now. What's your take on this @jnyrup ?

@lg2de
Copy link
Contributor

lg2de commented Sep 1, 2021

Many projects have activated "Warnung as Error". So, Obsolete will cause build errors. Therefore it should not be used here.
I think it is OK to add EditorBrowsable.
In Version 7 the old version can be removed. @dennisdoomen should we create an issue to collect all work shifted to V7? Or do you like to use "milestones"?

@gliljas, I think you should rebase to fix the conflicts.

@dennisdoomen
Copy link
Member

Yes, a milestone is fine. But we're not going to start on that anytime soon. Probably in two years or so.

@jnyrup
Copy link
Member

jnyrup commented Sep 1, 2021

I'm fine with the new overloads and only hiding the current ones.
Let's not add Obsolete - it would make me a bit sad if that scares/hinders people from upgrading to v6.

The release notes should have a line about the new overloads.

@dennisdoomen
Copy link
Member

and only hiding the current ones.

Wait, what do you mean with that? Because the latest commit removes them, which would be a breaking change.

@gliljas
Copy link
Contributor Author

gliljas commented Sep 1, 2021

and only hiding the current ones.

Wait, what do you mean with that? Because the latest commit removes them, which would be a breaking change.

I think he meant the EditorBrowsable thing, making use of the current methods somewhat discouraged. The current ones are still there, as far as I know/intended.

@dennisdoomen
Copy link
Member

EditorBrowsable was only used for UI designers. Why is it relevant here?

@jnyrup
Copy link
Member

jnyrup commented Sep 1, 2021

Adding that attribute will hide the current/old overload from intellisense suggestions in VS

@lg2de lg2de mentioned this pull request Sep 1, 2021
13 tasks
@dennisdoomen
Copy link
Member

Adding that attribute will hide the current/old overload from intellisense suggestions in VS

Nice. I didn't know that. Never too old to learn something new.

@dennisdoomen dennisdoomen added this to the 7.0 milestone Sep 2, 2021
@dennisdoomen dennisdoomen merged commit 42f0f8f into fluentassertions:master Sep 6, 2021
@dennisdoomen dennisdoomen removed this from the 7.0 milestone Sep 16, 2021
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.

None yet

4 participants