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

Added erase() test and new exception instead of just returning nothing #81

Closed
wants to merge 2 commits into from

Conversation

nexxai
Copy link

@nexxai nexxai commented May 2, 2020

Fixes #57

@nexxai
Copy link
Author

nexxai commented May 2, 2020

I don't understand why StyleCI is choking; the before and after text both look the same.

@SharakPL
Copy link

SharakPL commented May 2, 2020

You're using tabs instead of spaces. Besides you've increased indentation width from 4 to 8. That's against PSR.

Copy link
Owner

@bumbummen99 bumbummen99 left a comment

Choose a reason for hiding this comment

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

This is looking good, great work :)

@nexxai
Copy link
Author

nexxai commented May 2, 2020

Also, this will be a breaking change. If people attempt to restore a non-existent cart, they may be expecting it to just return nothing, rather than an exception. I'll fix this up right now and get it submitted.

@nexxai
Copy link
Author

nexxai commented May 2, 2020

@SharakPL Can you review my .php_cs and let me know which setting I should be changing to be PSR-compliant?

https://gist.github.com/nexxai/40388d76e88ac07cbf08e80c4d645d4c

@bumbummen99
Copy link
Owner

bumbummen99 commented May 2, 2020

@nexxai According to https://stackoverflow.com/questions/37193540/configure-php-cs-fixer-indentation-for-2-spaces-rather-than-4 you should simply replace the `->setIndent("\t") in your config with spaces

@nexxai
Copy link
Author

nexxai commented May 2, 2020

Cancelling; resubmitting.

@nexxai nexxai closed this May 2, 2020
@SharakPL
Copy link

SharakPL commented May 2, 2020

I recommend using PSR12. Just make it your default with phpcs --config-set default_standard PSR12.

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.

Create test for erase()
3 participants