Skip to content

Add several new methods to Types and Methods Assertions#1306

Merged
jnyrup merged 30 commits intofluentassertions:developfrom
fshchudlo:develop
Apr 27, 2020
Merged

Add several new methods to Types and Methods Assertions#1306
jnyrup merged 30 commits intofluentassertions:developfrom
fshchudlo:develop

Conversation

@fshchudlo
Copy link
Copy Markdown
Contributor

@fshchudlo fshchudlo commented Apr 17, 2020

Hello there.
I've added a couple of methods to types selectors and assertions which, I hope, you find useful.
Corresponding unit tests, of course, added.

Approval tests are definitely broken since I've added new methods, but I'm not sure if I should change them since they are approval tests. I would be grateful if you instruct me on this

IMPORTANT

  • The code complies with the Coding Guidelines for C#.
  • The changes are covered by a new or existing set of unit tests which follow the Arrange-Act-Assert syntax such as is used in this example.
  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.

@fshchudlo
Copy link
Copy Markdown
Contributor Author

fshchudlo commented Apr 21, 2020

@jnyrup @dennisdoomen, hi there
I've merged the conflict which I did not see when creating this PR. If I should do anything else to make this PR appropriate, reach out to me with any comments.

@dennisdoomen
Copy link
Copy Markdown
Member

dennisdoomen commented Apr 22, 2020

Sorry, completely missed this one. Will take a look tomorrow.

dennisdoomen
dennisdoomen approved these changes Apr 23, 2020
View reviewed changes
@dennisdoomen
Copy link
Copy Markdown
Member

dennisdoomen commented Apr 23, 2020

dennisdoomen approved these changes 12 minutes ago

Sorry, I meant to click the "Request Changes" button.

Approval tests are definitely broken since I've added new methods, but I'm not sure if I should change them since they are approval tests. I would be grateful if you instruct me on this

You can run AcceptApiChanges.ps1 to accept the changes and update the contract files.

Oh, and would you also be willing to update the docs and release notes under docs\pages?

@fshchudlo
Copy link
Copy Markdown
Contributor Author

fshchudlo commented Apr 23, 2020

Sure. it's a pleasure for me to make a contribution to such a nice library :)
I'll do everything needed ASAP.

@dennisdoomen dennisdoomen requested a review from jnyrup April 23, 2020 08:41
@fshchudlo
Copy link
Copy Markdown
Contributor Author

fshchudlo commented Apr 23, 2020 •
edited
Loading

Oh, and would you also be willing to update the docs and release notes under docs\pages?

I've described new ReturnTypes, UnwrapTaskTypes and UnwrapEnumerableTypes. Everything else is small changes that fit the description at the end of the docs: "There are so many possibilities and specialized methods that none of these examples do them good. Check out the TypeAssertionSpecs.cs".

Can you also instruct me about release notes? Should it be described as 5.11.0 or 6.1.0 version?

I need more time to add tests for TypeEnumerableExtensions. I hope I'll be able to add them tomorrow.

And the last one, I would be grateful if you instruct me about the need to rebase all these changes to a single commit since there are a lot of them now.

@dennisdoomen
Copy link
Copy Markdown
Member

dennisdoomen commented Apr 23, 2020

I've described new ReturnTypes, UnwrapTaskTypes and UnwrapEnumerableTypes. Everything else is small changes that fit the description at the end of the docs

Looks good to me.

Can you also instruct me about release notes? Should it be described as 5.11.0 or 6.1.0 version?

It's all going to be part of 6.0.0. Just add it under the "new" section.

And the last one, I would be grateful if you instruct me about the need to rebase all these changes to a single commit since there are a lot of them now.

If you have not "designed" your PR as clean separated commits, will just squash-merge your changes. I myself follow a pretty rigorous practice to get a clean source control history.

@fshchudlo
Copy link
Copy Markdown
Contributor Author

fshchudlo commented Apr 23, 2020

Well, as far as I can see, everything is done :)

But the build is broken for some reason not related to my changes, as I can see. Could you please look what’s the matter for it?

jnyrup
jnyrup requested changes Apr 24, 2020
View reviewed changes
@fshchudlo fshchudlo requested a review from jnyrup April 24, 2020 11:19
jnyrup
jnyrup requested changes Apr 24, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@jnyrup jnyrup left a comment

Choose a reason for hiding this comment

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

Love the feeling every time I get to digest a PR, to become more comfortable with it, and we together file of the rough edges to get a polished result ❤️

@fshchudlo fshchudlo requested a review from jnyrup April 27, 2020 13:31
@jnyrup jnyrup merged commit 48a3f07 into fluentassertions:develop Apr 27, 2020
@jnyrup
Copy link
Copy Markdown
Member

jnyrup commented Apr 27, 2020

@fshchudlo Thanks for the contribution!

@fshchudlo
Copy link
Copy Markdown
Contributor Author

fshchudlo commented Apr 27, 2020

@jnyrup I'm sure this is my first contribution, but not the last :) Thank you for such a thorough study of PR and such a cool library :)

@jnyrup jnyrup mentioned this pull request Oct 28, 2021
Closed
88 tasks
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

@fshchudlo @dennisdoomen @jnyrup