Skip to content

Provide an ability to override Exception class - #19

Merged
webmozart merged 2 commits into
webmozarts:masterfrom
JustBlackBird:custom_exception
Oct 10, 2016
Merged

Provide an ability to override Exception class#19
webmozart merged 2 commits into
webmozarts:masterfrom
JustBlackBird:custom_exception

Conversation

@JustBlackBird

@JustBlackBird JustBlackBird commented Sep 27, 2016

Copy link
Copy Markdown
Contributor

This PR is attended to fixes #17

Nyholm
Nyholm reviewed Sep 27, 2016
View reviewed changes
Comment thread src/Assert.php Outdated
return mb_strwidth($value, $encoding);
}

protected static function createException($message)

@Nyholm Nyholm Sep 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you considered a name like createInvalidArgumentException? We may (though unlikely) want to introduce ie LogicExceptions in the future. That would make a name like createException be too vague.

@JustBlackBird JustBlackBird Sep 27, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right! I did not think about this possibility and will fix the name.

@JustBlackBird JustBlackBird Sep 27, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

@JustBlackBird

JustBlackBird commented Sep 29, 2016

Copy link
Copy Markdown
Contributor Author

Should I fix anything else to get the PR merged?

@webmozart

webmozart commented Oct 10, 2016

Copy link
Copy Markdown
Member

Hi :) I don't see a problem with that change. Thanks!

@webmozart
webmozart merged commit d5b00dc into webmozarts:master Oct 10, 2016
@emodric

emodric commented Oct 11, 2016

Copy link
Copy Markdown

@webmozart When do you plan to release the new version with this change?

@webmozart

webmozart commented Oct 11, 2016

Copy link
Copy Markdown
Member

@emodric As soon as #18 is through I'll tag a new release.

@keradus

keradus commented Oct 12, 2016

Copy link
Copy Markdown
Contributor

I miss new release as well ;)
@webmozart , you could always have two releases ;)

@keradus keradus mentioned this pull request Oct 12, 2016
Merged
@JustBlackBird
JustBlackBird deleted the custom_exception branch November 25, 2016 21:22
@deployable

deployable commented Jun 23, 2017

Copy link
Copy Markdown

Is this PR going to be released? I can't see the createException function in the 1.2.0 release even though that release was tagged in Nov 2016 after the merge.

@keradus

keradus commented Jun 23, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

@deployable it was never released, it was modified with #22 and then release together.
use reportInvalidArgument instead

@deployable

deployable commented Jun 30, 2017

Copy link
Copy Markdown

@keradus I see, thanks!

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

Reviewers

1 more reviewer
@Nyholm Nyholm Nyholm left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Proposal: Custom exception class

6 participants

@JustBlackBird @webmozart @emodric @keradus @deployable @Nyholm