Found function services_node_load() in file services.runtime.inc that is never used. Lets remove it.

CommentFileSizeAuthor
services-remove-services_node_load.patch800 bytesygerasimov

Comments

kylebrowning’s picture

kylebrowning commented

What if other modules use it?

Status: Needs review » Needs work

The last submitted patch, services-remove-services_node_load.patch, failed testing.

ygerasimov’s picture

ygerasimov commented

I think we should not keep the code that we do not use just because of other modules "might" use it. This function from name is "node" specific and sits in runtime file (that is illogical anyway). So I think this is left over from d6 version or something. If even someone uses it -- it will be trivial to restore it for them.

I have seen this kind of break of API's in other modules (apachesolr particular, but I can't give you exact fact as it was like a year ago) and that was completely ok.

ygerasimov’s picture

ygerasimov commented
Status: Needs work » Needs review

services-remove-services_node_load.patch queued for re-testing.

marcingy’s picture

marcingy commented

This is definitely old stuff that was left lying around from the 6.x.2 branch.

ygerasimov’s picture

ygerasimov commented
Status: Needs review » Fixed

Committed to both 7.x-3.x and 6.x-3.x.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

marabak’s picture

marabak commented

services_node_load() function was useful to load light nodes, because of the second argument $fields, we could specify only the fields we wanted. And that was a feature provided only by services.

stongo’s picture

stongo commented
Issue summary: View changes

Sad about this. See https://drupal.org/node/1825800#comment-8504221

tyler.frankenstein’s picture

tyler.frankenstein commented

@stongo, I've made a patch for the sadness:

https://drupal.org/node/2229489#comment-8635097

c-logemann’s picture

c-logemann
he/him
Primary language German
Location Frankfurt/M, Germany
commented
Status: Closed (fixed) » Active

I am just working on D6 tests and just found this issue because I was searching for "services_node_load". This is missing by the test "Resource Taxonomy" in D6.
@ygerasimov: Please don't remove code from D6 branch without testing the D6 version itself.

I think this little bit of code doesn't hurt the D6 version. I like to simple fix the tests of the D6 Version. And as reported in other comments there are people out there missing it, too.

@kylebrowning: What about just reverting this commit? I have created I pull request on github for you.

marcingy’s picture

marcingy commented
Status: Active » Closed (fixed)

This was re-opened for the d6 branch this won't ever be fixed