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

Spec: restrict the type returned by should_not be_nil and others #8412

Merged
merged 5 commits into from
Nov 8, 2019

Conversation

asterite
Copy link
Member

Fixes #8369

Follow up to #8240

@asterite asterite force-pushed the spec/restrict-types branch from a017286 to 25c7247 Compare November 8, 2019 12:07
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

This is technically missing should be_nil. I assume that's on purpose because it's hard to think of a use case for this?

@@ -417,6 +417,26 @@ module Spec
end

module ObjectExtensions
# Validates an expectation and fails the example if it does not match.
#
# This overload will be called when doing:
Copy link
Member

Choose a reason for hiding this comment

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

I'd avoid placing an example block inside a sentence.

Suggested change
# This overload will be called when doing:
# This overload accepts a `be_a(Type)` expectation and makes sure, at compile-time, that the returned object is of type `Type`.

Copy link
Member

Choose a reason for hiding this comment

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

Any reason for not mentioning the be_a method here? It would make the documentation more accessible for readers familiar with the DSL.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, what do you mean? I changed the code example and be_a is used there.

Copy link
Member

Choose a reason for hiding this comment

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

I meant mentioning it in the text.

But yeah, having it in the sample is probably fine.

@asterite
Copy link
Member Author

asterite commented Nov 8, 2019

This is technically missing should be_nil. I assume that's on purpose because it's hard to think of a use case for this?

Right, the resulting type will always be nil and afterwards there's not much you can do with that.

@asterite asterite force-pushed the spec/restrict-types branch from 2cc5802 to 945d4bf Compare November 8, 2019 15:14
@asterite
Copy link
Member Author

asterite commented Nov 8, 2019

Fixed once more. Let me know if this is good now.

Co-Authored-By: Johannes Müller <[email protected]>
asterite and others added 2 commits November 8, 2019 13:59
@asterite asterite added this to the 0.32.0 milestone Nov 8, 2019
@asterite asterite merged commit bb5bb88 into crystal-lang:master Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spec: cast values for be_nil and be_a expectations
3 participants