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.

Sign up for GitHub

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

Jump to bottom

ArrayValidator clean up #4309

Merged
merged 2 commits into from
May 24, 2019
Merged

ArrayValidator clean up #4309

merged 2 commits into from
May 24, 2019

Conversation

timkelty
Copy link
Contributor

I was extending ArrayValidator and this threw me for a loop…I certainly could be missing something, but I couldn't figure out why it was written this way.

  • The validation logic was duplicated in both validateAttribute and validateValue
  • validateValue was actually never called, as it is protected and not called from validateAttribute (unless something else extended ArrayValidator)
  • the !is_string($value) check in validateAttribute seemed backward

@timkelty timkelty changed the title Move logic to validateValue, which will actually get used now… ArrayValidator clean up May 24, 2019
@brandonkelly brandonkelly merged commit 9d3976c into craftcms:3.2 May 24, 2019
@brandonkelly
Copy link
Member

Thanks! No idea what I was thinking there with validateValue() 🤷‍♂️

brandonkelly added a commit that referenced this pull request May 24, 2019
@brandonkelly
Release note for #4309
3a56d98
@timkelty timkelty mentioned this pull request May 24, 2019
Closed
angrybrad pushed a commit that referenced this pull request May 27, 2019
@brandonkelly @angrybrad
Release note for #4309
dbce457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@timkelty @brandonkelly