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

Validation #22

Merged
merged 31 commits into from
Feb 7, 2020
Merged

Validation #22

merged 31 commits into from
Feb 7, 2020

Conversation

ondrejbouda
Copy link
Contributor

No description provided.

Copy link
Contributor

@xificurk xificurk left a comment

Choose a reason for hiding this comment

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

Making the final on message and handler optional opens a whole new can of worms and I'm not sure if that's wise...
Commands and/or handlers can now inherit from each other (which would probably break handler resolver logic). Also, they can be abstract and there is no validation for this.

src/StaticAnalysis/Rules/MethodHasOneParameterRule.php Outdated Show resolved Hide resolved
src/StaticAnalysis/Rules/MethodReturnTypeIsInRule.php Outdated Show resolved Hide resolved
src/StaticAnalysis/Rules/ClassNameHasSuffixRule.php Outdated Show resolved Hide resolved
src/StaticAnalysis/Rules/ClassNameHasSuffixRule.php Outdated Show resolved Hide resolved
src/StaticAnalysis/Rules/ShortClassNameMatchesRule.php Outdated Show resolved Hide resolved
@ondrejbouda
Copy link
Contributor Author

In my previous commits I added support for custom validators.

@ondrejbouda
Copy link
Contributor Author

Making the final on message and handler optional opens a whole new can of worms and I'm not sure if that's wise...
Commands and/or handlers can now inherit from each other (which would probably break handler resolver logic). Also, they can be abstract and there is no validation for this.

This hole is already open. There is nothing that would make you to even use the validators in the first place. I suggest to let this be for now. The default is alright (strict pre-configured validators for commands and events), any other scenario is up to the user.

Maybe we could come up with a solution that somehow ties the validation logic and handler-resolving logic. But for now, I would leave it be.

@ondrejbouda ondrejbouda merged commit 1f67c3b into master Feb 7, 2020
@ondrejbouda ondrejbouda deleted the validation branch February 7, 2020 11:41
@ondrejbouda ondrejbouda mentioned this pull request Feb 7, 2020
29 tasks
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.

2 participants