Skip to content

Conversation

@andersio
Copy link
Member

@andersio andersio commented Jun 5, 2017

Raise warnings when using observation API or operators that is essentially a logic error due to Value or Error being an inhabitable type. Please refer to InhabitableTypeGuards.swift for the lists of affected operators.

Also note that these overloads cannot be marked as unavailable or obsolete, or otherwise the regular version would take precedence over this.

Checklist

  • Updated CHANGELOG.md.

@andersio andersio requested a review from mdiep June 5, 2017 11:37
@andersio andersio changed the title Warning for observation APIs when used with inhabitable types. Warning for observation APIs when used with inhabitable types & promoteValue. Jun 6, 2017
Signal<Never, NoError>.never.flatMapError { _ in SignalProducer<Never, AnyError>.empty }
Signal<Never, NoError>.never.flatMapError { _ in SignalProducer<Never, NoError>.empty }
}
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this?

Copy link
Member Author

@andersio andersio Jun 7, 2017

Choose a reason for hiding this comment

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

Just a bunch of test cases to verify these combinations would emit the error message. Moving them to the spec would flood the IDE with warnings, so I just commented them out. 🤦‍♂️

@mdiep
Copy link
Contributor

mdiep commented Jun 7, 2017

Just needs a changelog entry or two.

@andersio
Copy link
Member Author

andersio commented Jun 7, 2017

I think it needs better error message to tell that ignoring the error message would trap at runtime & also fill in the blanks for fatalError.

@andersio
Copy link
Member Author

andersio commented Jun 8, 2017

@mdiep Please check. The error message are written in reduced written register btw, following the swift stdlib style.

@mdiep mdiep merged commit 86d9f62 into master Jun 8, 2017
@mdiep mdiep deleted the never-guard branch June 8, 2017 13:22
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.

3 participants