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

Return error code with RuntimeException. #6

Merged
merged 4 commits into from
Apr 24, 2021

Conversation

EreMaijala
Copy link
Contributor

@EreMaijala EreMaijala commented Feb 18, 2021

This brings parity with other storage adapters that already include the error code in exceptions.

Signed-off-by: Ere Maijala ere.maijala@helsinki.fi

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC no
QA no

src/Memcached.php Show resolved Hide resolved
@boesing boesing added this to the 1.1.0 milestone Feb 18, 2021
@boesing
Copy link
Member

boesing commented Feb 18, 2021

I'll add GHA in a PR this weekend. I'll try to rebase this PR afterwards so we can verify the test results here. 👍🏼

EreMaijala and others added 2 commits April 24, 2021 18:52
This brings parity with other storage adapters that already include the error code in exceptions.

Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing force-pushed the 1.1.x-exception-code branch from 6ed5716 to 03ff1ef Compare April 24, 2021 17:09
@boesing
Copy link
Member

boesing commented Apr 24, 2021

Took me a bit but I've added GHA now, added a unit test for your changes and made the tests pass. 👍🏼
Thanks, @EreMaijala!

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing force-pushed the 1.1.x-exception-code branch from 94f3641 to 6cbf0a9 Compare April 24, 2021 19:43
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing
Copy link
Member

boesing commented Apr 24, 2021

@EreMaijala For integrity, I've passed the result code to the other RuntimeException creations as well and created tests for that.

@boesing boesing merged commit f5d35cc into laminas:1.1.x Apr 24, 2021
@EreMaijala
Copy link
Contributor Author

@boesing Thanks, I appreciate your extra work and attention to detail. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants