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

Make Expectation constructor public #531

Merged
merged 1 commit into from
Jun 12, 2018
Merged

Make Expectation constructor public #531

merged 1 commit into from
Jun 12, 2018

Conversation

mrcljx
Copy link
Contributor

@mrcljx mrcljx commented Jun 5, 2018

Some libraries (like RxNimble) create their own expectations. The following excerpt from RxNimble looks like a workaround to the lack of a public constructor:

extension Expectation {
    init(_ expression: Expression<T>) {
        self.expression = expression
    }

    // ...
}

But in current Swift versions this does not work anymore:

Initializer for struct 'Expectation' must use "self.init(...)" or "self = ..." because it is not in module 'Nimble'

That of course is not possible as the constructor is not public.


Not sure if this should be a patch or minor version bump.

Some libraries (like `RxNimble`) create their own expectations.

This [extension from RxNimble](https://github.com/RxSwiftCommunity/RxNimble/blob/602c14aae95db274004cc77ff1779a5feaf6f6fc/Source/Expectation%2BExt.swift#L4-L6) seems to have worked in earlier versions of Swift but not anymore:

> Initializer for struct 'Expectation<T>' must use "self.init(...)" or "self = ..." because it is not in module 'Nimble'
@mrcljx
Copy link
Contributor Author

mrcljx commented Jun 5, 2018

This stopped working in Swift 4.1 because of SE-0189: Restrict Cross-module Struct Initializers.

Copy link
Member

@ashfurrow ashfurrow left a comment

Choose a reason for hiding this comment

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

Makes sense to me! Will wait for another member of the team to merge.

@mrcljx
Copy link
Contributor Author

mrcljx commented Jun 7, 2018

Dropped the checkboxes from the PR description so that it looks more mergeable from the list.

@mrcljx
Copy link
Contributor Author

mrcljx commented Jun 11, 2018

@ashfurrow Is there anyone I could ping on this?

@ashfurrow
Copy link
Member

@Quick/core This looks good to go, pretty small, additive change. Any objections to merging?

Copy link
Member

@ikesyo ikesyo left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

@c0diq
Copy link

c0diq commented Aug 28, 2018

Could we get a new release to get this fix please?

@ikesyo
Copy link
Member

ikesyo commented Aug 28, 2018

Please follow #575.

Megal pushed a commit to Megal/Nimble that referenced this pull request Jul 31, 2019
Make Expectation constructor public
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