Closed (fixed)
Project:
Search API
Version:
8.x-1.x-dev
Component:
Framework
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
7 Aug 2015 at 19:03 UTC
Updated:
13 Oct 2015 at 15:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
lhangea commentedComment #3
lhangea commentedComment #5
lhangea commentedComment #7
lhangea commentedComment #8
oenie commentedI think you need to make sure you generate your patch in the root drupal directory, to make sure it is properly generated.
The testrunner is tripping over the paths in your patch, which are relative to the directory of the search_api module.
Comment #9
lhangea commentedComment #11
oenie commentedAre you inside one of the rooms in Barcelona ?
If you feel like it, i could come by. I'm in the General Sprints room.
Let me know which room/table :)
Comment #12
lhangea commentedComment #13
lhangea commentedComment #14
lhangea commentedComment #18
drunken monkeyThanks a lot for your work here, this already looks excellent, and you also seem to have caught all of them!
Just a few minor details:
That seems to be left over from an earlier patch?
There is actually already
$this->getEntityStorage()for that in this class.Now this is just my pedantic nit-picking about coding standards, but property names should actually be camelCase, not snake_case.
But good idea to separate this into its own property instead of having that code in numerous places.
Also coding style nit-picking, but even more specific: we're currently not using the shorthand array syntax in the Search API, and I'd like to remain consistent in that aspect.
Attached is an updated patch with those few problems removed. Also, while we're at it, I also went ahead and removed all the other calls to
entity_*()functions (which are now all deprecated, it seems).Sorry, didn't notice that earlier.
Comment #21
drunken monkeyForgot to remove the left-over database backend change in the previous patch.
Comment #24
drunken monkeyThat's the kind of stuff that happens if you try to be clever at midnight …
Comment #27
lhangea commentedAll right, I think I found the source of the exception.
Comment #28
lhangea commentedComment #30
lhangea commentedI missed a parenthesis.
Comment #32
lhangea commented@oenie yes I was at DrupalCon sprints, sorry I didn't see your comment from #11.
Comment #36
drunken monkeyGreat job, thanks! And you even changed the property name I forgot to align with the coding standards, nice.
So, committed.
Thanks a lot again!