Skip to content

Conversation

@sngrl
Copy link

@sngrl sngrl commented Feb 9, 2024

No description provided.

@sngrl sngrl requested a review from tillkruss as a code owner February 9, 2024 22:10
@coveralls
Copy link

Coverage Status

coverage: 80.265% (+0.06%) from 80.208%
when pulling 294de23 on sngrl:patch-1
into d44f073 on predis:v2.x.

@vladvildanov
Copy link
Contributor

@sngrl Would you mind to add a test coverage? Literally, it's just about extending already existing data provider in unit tests

@sngrl2
Copy link

sngrl2 commented Feb 12, 2024

@vladvildanov tests already are presented, my PR just makes this commands to use prefixes

\Predis\Command\Redis\ZPOPMIN_Test
\Predis\Command\Redis\ZPOPMAX_Test

vladvildanov
vladvildanov previously approved these changes Feb 13, 2024
@vladvildanov
Copy link
Contributor

@sngrl2 Ah, my bad! Approved

But you also need to run composer style:fix command to fix codestyle issues

@tillkruss tillkruss assigned vladvildanov and unassigned tillkruss Apr 14, 2025
@tillkruss tillkruss changed the title ZPOPMIN & ZPOPMAX with "prefix first" option added to the KeyPrefixProcessor ZPOPMIN & ZPOPMAX prefixes Apr 14, 2025
@tillkruss tillkruss changed the title ZPOPMIN & ZPOPMAX prefixes [2.x] ZPOPMIN & ZPOPMAX prefixes Apr 14, 2025
@coveralls
Copy link

coveralls commented Apr 15, 2025

Coverage Status

coverage: 88.825%. remained the same
when pulling 5df852f on sngrl:patch-1
into 9418df1 on predis:v2.x.

@vladvildanov
Copy link
Contributor

@tillkruss Ready to merge

@tillkruss
Copy link
Member

@vladvildanov Did you undo the commit? 🫠

@vladvildanov
Copy link
Contributor

@tillkruss I didn't, this branch was outdated and when I sync it it turns that this changes was done as a part of another commit, so tests are the only thing that were missing

@tillkruss tillkruss merged commit 1b5ed7d into predis:v2.x Apr 15, 2025
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants