Skip to content

Split Type assertions in separate files #1576

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.

Sign up for GitHub

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

Jump to bottom
Merged
dennisdoomen merged 1 commit into from
May 30, 2021
Merged

Split Type assertions in separate files #1576

dennisdoomen merged 1 commit into from
May 30, 2021

Conversation

@eNeRGy164
Copy link
Contributor

@eNeRGy164 eNeRGy164 commented May 24, 2021

As discussed in #1551

  • Put [Not] in filename to make clear both version are in the same file
  • Kept regions to make group positive and negative (not) methods
  • Kept generic variants with the non-generic methods
  • Except for the main file, added <content> instead of <summary> to make sure StyleCop rules were passing, but any documentation (unlikely) would come from a single source.
  • Discovered the NotHavePropertyOfT specs are missing

IMPORTANT

@dennisdoomen
Copy link
Member

dennisdoomen commented May 26, 2021

I like the way the files are split, but I'm not sure I like the [Not] in the name. Option 1 did not have that specific element. What do you think @jnyrup? Do you think it adds value?

@eNeRGy164
Copy link
Contributor Author

eNeRGy164 commented May 26, 2021

I can change it of course.

When organizing, I was thinking how logical it is for anybody looking at the code independent of IDE features that both the positive and negative version are in the same file.

I was imagining several ways (that are allowed by the filesystem).
I settled on [Not] as that is also the notation used in the documentation/release notes.

So, the file containing: HaveImplicitConversionOperator, HaveImplicitConversionOperator<T>, NotHaveImplicitConversionOperator and NotHaveImplicitConversionOperator<T> is called...

TypeAssertionSpecs.HaveImplicitConversionOperator.cs 
TypeAssertionSpecs.[Not]HaveImplicitConversionOperator.cs 
TypeAssertionSpecs.Not-HaveImplicitConversionOperator.cs 
TypeAssertionSpecs.Not.HaveImplicitConversionOperator.cs 
TypeAssertionSpecs.Not_HaveImplicitConversionOperator.cs

@dennisdoomen
Copy link
Member

dennisdoomen commented May 26, 2021

Yes, I understand, but I don't quite like it myself.

@jnyrup
Copy link
Member

jnyrup commented May 29, 2021

I would prefer if we just left out the [Not].
Other than that I like the changes here 👍

@eNeRGy164
Copy link
Contributor Author

eNeRGy164 commented May 29, 2021

I'll change it. Downside is that the Be comes before cs
image

@dennisdoomen dennisdoomen merged commit c7de3c8 into fluentassertions:develop May 30, 2021
@eNeRGy164 eNeRGy164 deleted the split-type-specs branch May 30, 2021 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dennisdoomen dennisdoomen dennisdoomen approved these changes

@jnyrup jnyrup jnyrup approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eNeRGy164 @dennisdoomen @jnyrup