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

fix #318 #488

Merged
merged 3 commits into from
Aug 4, 2021
Merged

fix #318 #488

merged 3 commits into from
Aug 4, 2021

Conversation

hutterm
Copy link
Contributor

@hutterm hutterm commented Jun 28, 2021

InnerJoin implementation similar to #483

This might be an even more breaking change since the key is now a tuple...

@hutterm
Copy link
Contributor Author

hutterm commented Jun 28, 2021

don't know why this test fails, locally it passes

@hutterm
Copy link
Contributor Author

hutterm commented Jul 5, 2021

... also the failing test isn't even related to my changes

@hutterm
Copy link
Contributor Author

hutterm commented Jul 30, 2021

@RolandPheasant Opinions on this?

@RolandPheasant
Copy link
Collaborator

Sorry ain't got around to looking yet. Will do so over the weekend.

@@ -14,119 +14,119 @@ internal class InnerJoin<TLeft, TLeftKey, TRight, TRightKey, TDestination>
{
private readonly IObservable<IChangeSet<TLeft, TLeftKey>> _left;

private readonly Func<TLeftKey, TLeft, TRight, TDestination> _resultSelector;
private readonly Func<(TLeftKey leftKey, TRightKey rightKey), TLeft, TRight, TDestination> _resultSelector;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes perfect sense to me

@RolandPheasant
Copy link
Collaborator

I am happy to merge this but before we release, we need a wiki page to explain the breaking change and why. Are you happy to do that?

@hutterm
Copy link
Contributor Author

hutterm commented Aug 4, 2021

@RolandPheasant
Copy link
Collaborator

@RolandPheasant RolandPheasant merged commit e17346c into reactivemarbles:main Aug 4, 2021
@hutterm hutterm deleted the fixinnerjoin branch August 4, 2021 15:46
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants