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

Selection::getPreviousAccessedColumns must return array or null [Closes #160] #161

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Selection::getPreviousAccessedColumns must return array or null [Closes #160] #161

wants to merge 1 commit into from

Conversation

hranicka
Copy link
Contributor

@hranicka hranicka commented Feb 2, 2017

  • bug fix? yes
  • new feature? no
  • BC break? no

Just fixed annotation + array return typehint for Selection::getPreviousAccessedColumns.

Fixes #160

@dg
Copy link
Member

dg commented Feb 2, 2017

What about change FALSE to NULL via return array_keys(array_filter((array) $this->previousAccessedColumns)) ?: NULL? It will be consistent with other similar changes in framework.

@hranicka hranicka force-pushed the hotfix/prev-acc-cols-return branch from 0e858b7 to 965148e Compare February 2, 2017 12:32
@hranicka
Copy link
Contributor Author

hranicka commented Feb 2, 2017

@dg Right, it's fine. I've changed it, master :-)

@hranicka hranicka changed the title Selection::getPreviousAccessedColumns must return array [Closes #160] Selection::getPreviousAccessedColumns must return array or null [Closes #160] Feb 2, 2017
@hranicka
Copy link
Contributor Author

hranicka commented Feb 2, 2017

Now, tests are failing. Previous changes (return an empty array instead of FALSE) were OK: https://travis-ci.org/nette/database/builds/197590334

@dg dg force-pushed the master branch 4 times, most recently from abe9849 to 9358712 Compare June 29, 2017 15:38
@dg dg force-pushed the master branch 7 times, most recently from f1a9aa2 to ec7a207 Compare July 26, 2017 21:39
@foxycode foxycode mentioned this pull request Oct 11, 2017
Merged
@dg dg force-pushed the master branch 9 times, most recently from 5e7169d to 49cbe22 Compare March 23, 2018 15:57
@dg dg force-pushed the master branch 29 times, most recently from a329d1b to 9f82d9d Compare September 5, 2024 02:37
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.

Selection::getPreviousAccessedColumns invalid return annotation
2 participants
@hranicka @dg