Skip to content

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.

@gliljas gliljas force-pushed the greaterthanorequalto branch from 1834bb0 to ac48e5d Compare September 1, 2021 09:02
@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 gliljas force-pushed the greaterthanorequalto branch from ac48e5d to dbd54c1 Compare September 1, 2021 09:21
@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
14 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 requested a review from jnyrup September 4, 2021 07:03
@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.

4 participants