Skip to content

Feat: add iterable_merge function #45

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

Merged
merged 3 commits into from
Oct 15, 2022
Merged

Feat: add iterable_merge function #45

merged 3 commits into from
Oct 15, 2022

Conversation

jdecool
Copy link
Contributor

@jdecool jdecool commented Oct 2, 2022

Add iterable_merge function.

@jdecool jdecool marked this pull request as ready for review October 2, 2022 19:36
@codecov
Copy link

codecov bot commented Oct 10, 2022

Codecov Report

Merging #45 (085c417) into master (95571ed) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #45   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines           44        59   +15     
=========================================
+ Hits            44        59   +15     
Impacted Files Coverage Δ
src/IterableObject.php 100.00% <100.00%> (ø)
src/iterable-functions.php 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Owner

@bpolaszek bpolaszek left a comment

Choose a reason for hiding this comment

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

Hi @jdecool,

Thanks for your contribution & sorry for the late reply!

The following case does not succeed:

$array1 = ['bar', 'baz'];
$array2 = ['bat', 'baz'];
$generator1 = fn () => yield from $array1;
$generator2 = fn () => yield from $array2;

expect(iterable_merge($generator1(), $generator2())->asArray())->toEqual(array_merge($array1, $array2));

@jdecool
Copy link
Contributor Author

jdecool commented Oct 10, 2022

No problem @bpolaszek

I've just pushed some updates.

@bpolaszek
Copy link
Owner

Sounds good to me 🙂

Can you please fix CI + advertise it in README.md?

Thanks! 🙏

@jdecool
Copy link
Contributor Author

jdecool commented Oct 13, 2022

Can you please fix CI + advertise it in README.md?

Sure, everything should be OK now.

@jdecool
Copy link
Contributor Author

jdecool commented Oct 14, 2022

😮 it fails.

Sorry i’ve missed one check

@jdecool
Copy link
Contributor Author

jdecool commented Oct 14, 2022

Everything is fine now is the CI 😌

@bpolaszek bpolaszek merged commit 7136af2 into bpolaszek:master Oct 15, 2022
@bpolaszek
Copy link
Owner

Thanks @jdecool, your contribution is very much appreciated! 👍

@jdecool jdecool deleted the feat-iterable-merge branch October 15, 2022 07:12
/**
* Merge iterables
*
* @param iterable<TKey, TValue> ...$args
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW I think this template will not work for merging iterables with different value types.

Also it seems, there's no support for typing it properly phpstan/phpstan#3814 (comment)

@JoshuaBehrens
Copy link

Nice feature :)

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