Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Relays+Infallible #2430

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

Open
mashe wants to merge 3 commits into ReactiveX:main
base: main
Choose a base branch
Loading
from mashe:relays-asInfallible()
Open

Relays+Infallible #2430

mashe wants to merge 3 commits into ReactiveX:main from mashe:relays-asInfallible()

Conversation

@mashe
Copy link

@mashe mashe commented Jun 18, 2022

Copy link
Member

@freak4pc freak4pc left a comment

Choose a reason for hiding this comment

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

Lets get some tests on this and we can merge it in.
Thanks!

}
}

public extension PublishRelay {
Copy link
Member

@freak4pc freak4pc Aug 5, 2022

Choose a reason for hiding this comment

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

Definitely needs some test coverage :)

Copy link
Member

@freak4pc freak4pc Aug 5, 2022

Choose a reason for hiding this comment

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

(basically just something that compares the results of the relay to the infallible is "good enough" here).

Copy link
Member

freak4pc commented Aug 8, 2022

@mashe got the time to take this to the finishing line? 🙏

Copy link

This should just replace the relay specific parts of #2401.
Basically, just replace asInfallible(onErrorFallbackTo: .empty()) in the methods added in that PR with Infallible(self.asObservable())

TTOzzi reacted with thumbs up emoji

Copy link
Contributor

TTOzzi commented Aug 11, 2022

It already has test cases in Infallible+Tests.swift 🤔

Copy link
Author

mashe commented Aug 14, 2022

Thank you, everyone for keeping eye on the PR.
@freak4pc, I have followed @rtharston suggestion, so I reverted initial commit and updated existing .asInfallible() functions for Relays. I believe additional tests are not required, since @TTOzzi noted that those exist already.

TTOzzi and PandaKris reacted with thumbs up emoji

Copy link

@freak4pc, any update on this?

@freak4pc freak4pc force-pushed the main branch 2 times, most recently from 1198683 to 5949cbd Compare April 21, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@freak4pc freak4pc freak4pc requested changes

+3 more reviewers

@volovely volovely volovely approved these changes

@TTOzzi TTOzzi TTOzzi approved these changes

@JamesNguyenSotatek JamesNguyenSotatek JamesNguyenSotatek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /