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
+3 −3

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
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 2 times, most recently from 89f161f to b2e171f Compare October 10, 2023 16:59
@dg dg force-pushed the master branch 2 times, most recently from 893ffa2 to 4c9eb34 Compare November 5, 2023 19:45
@dg dg force-pushed the master branch 7 times, most recently from 2f4eae7 to b8686b9 Compare December 8, 2023 11:17
@dg dg force-pushed the master branch 7 times, most recently from ed576c5 to e27cfc8 Compare December 14, 2023 20:02
@dg dg force-pushed the master branch 2 times, most recently from 13dc813 to 61e494f Compare May 2, 2024 10:40
@dg dg force-pushed the master branch 6 times, most recently from 53f52b2 to 860a033 Compare May 13, 2024 16:16
@dg dg force-pushed the master branch 2 times, most recently from 458832d to c328e2c Compare May 16, 2024 14:01
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