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.

Sign up for GitHub

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

Jump to bottom

Refactor ::partition() and ::span() #124

Merged
merged 9 commits into from
Jul 8, 2021
Merged

Refactor ::partition() and ::span() #124

merged 9 commits into from
Jul 8, 2021

Conversation

drupol
Copy link
Collaborator

@drupol drupol commented Jul 6, 2021 •
edited by AlexandruGG
Loading

This PR

  • Refactor ::partition() operation
  • Refactor ::span() operation
  • Has updated tests
  • Has updated documentation
  • Has static analysis tests

Depends/Follow on #125
Fixes #123

@drupol drupol changed the title Refactor ::partition() and ::span()` Refactor ::partition() and ::span() Jul 6, 2021
@drupol drupol force-pushed the 123-refactor-partition-and-span branch 2 times, most recently from d97d11b to 46bf746 Compare July 6, 2021 15:38
@drupol drupol changed the base branch from master to feature/add-reject-operation July 6, 2021 15:41
@drupol drupol force-pushed the 123-refactor-partition-and-span branch from 74d1f90 to 200bf0b Compare July 6, 2021 16:01
@drupol drupol force-pushed the feature/add-reject-operation branch 2 times, most recently from cb8ef1e to c796e59 Compare July 6, 2021 17:06
@drupol drupol force-pushed the 123-refactor-partition-and-span branch 3 times, most recently from 6a0ecd8 to 1b263b7 Compare July 6, 2021 19:31
@drupol drupol marked this pull request as ready for review July 6, 2021 19:39
@drupol drupol requested a review from AlexandruGG July 6, 2021 19:39
@drupol drupol added the enhancement New feature or request label Jul 6, 2021
@AlexandruGG
Copy link
Collaborator

Really nice getting this up so quickly! I will focus on the other PRs first so we can get those merged

@drupol drupol force-pushed the feature/add-reject-operation branch 2 times, most recently from 71d0732 to 99c12d3 Compare July 7, 2021 06:56
Base automatically changed from feature/add-reject-operation to master July 7, 2021 08:08
@drupol drupol force-pushed the 123-refactor-partition-and-span branch 3 times, most recently from d29da2c to bef7ace Compare July 7, 2021 16:23
@drupol drupol force-pushed the 123-refactor-partition-and-span branch from bef7ace to 2fbfe5e Compare July 7, 2021 16:41
@drupol
Copy link
Collaborator Author

drupol commented Jul 7, 2021

I found a way to optimize the thing and remove the entries I was about to commit in psalm-baseline.xml !

@drupol drupol marked this pull request as draft July 7, 2021 16:57
@drupol drupol marked this pull request as ready for review July 7, 2021 17:26
@drupol drupol marked this pull request as draft July 7, 2021 18:50
@drupol drupol force-pushed the 123-refactor-partition-and-span branch from eae5891 to 0feb1f2 Compare July 7, 2021 18:57
@drupol drupol force-pushed the 123-refactor-partition-and-span branch from 0feb1f2 to 5eeb1ec Compare July 7, 2021 18:59
@drupol drupol marked this pull request as ready for review July 7, 2021 19:08
@drupol
Copy link
Collaborator Author

drupol commented Jul 7, 2021

Sorry for the back'n'forth, I don't touch this anymore now!

@AlexandruGG
Copy link
Collaborator

Sorry for the back'n'forth, I don't touch this anymore now!

It's okay I hadn't reviewed yet 😄 . There is no change on Span here other than the docs, did you decide to keep it for a separate PR or does nothing need to be refactored?

@drupol
Copy link
Collaborator Author

drupol commented Jul 7, 2021

The raw span operation is properly done, I just had to map the iterators to collection in Collection.php and it's good !

@AlexandruGG
Copy link
Collaborator

The raw span operation is properly done, I just had to map the iterators to collection in Collection.php and it's good !

Sounds good, thanks for moving so fast on this. Leave this to me then I will review and add the static analysis checks

@drupol
Copy link
Collaborator Author

drupol commented Jul 7, 2021

Sounds good :)💪👍

@AlexandruGG
Copy link
Collaborator

@drupol this should be ready once the mutation tests finish (the Scrutinizer failure seems unrelated). I polished the documentation a bit, added a few more tests, and added the static analysis checks

@drupol drupol merged commit 3543001 into master Jul 8, 2021
@drupol drupol deleted the 123-refactor-partition-and-span branch July 8, 2021 21:03
@drupol
Copy link
Collaborator Author

drupol commented Jul 8, 2021

All good, thanks !

@AlexandruGG
Copy link
Collaborator

All good, thanks !

Nice, we're gonna have another big release soon 😄

@drupol
Copy link
Collaborator Author

drupol commented Jul 8, 2021

Ho yes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexandruGG AlexandruGG AlexandruGG left review comments

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Partition Operation - Awkward to use?
2 participants
@drupol @AlexandruGG