-
Notifications
You must be signed in to change notification settings - Fork 115
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
NEW: Better type default. #830
NEW: Better type default. #830
Conversation
Thanks for the PR @mfendeksilverstripe. You'll need to target I believe that we didn't use |
20d0d4b
to
0368154
Compare
@ScopeyNZ Rebased on |
People might avoid this PR due to this comment. Let's see if @robbieaverill remembers ;P |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a recommended change - would this work for you?
91f3d94
to
6c9654f
Compare
6c9654f
to
728e4ee
Compare
This is ready for review @emteknetnz |
Looking through previous comments + code I think the reason not to use However @mfendeksilverstripe there's a PHPCS issue to fix before merging https://app.travis-ci.com/github/silverstripe/silverstripe-elemental/jobs/544323711#L1298 |
@emteknetnz Thanks for feedback, I've pushed up the linting fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Travis failure in PHP8 behat is existing
Better type default for BaseElement
Type
onBaseElement
singular_name
)Related issues
#831