This module is a dependency for the REST server that ships with the Services module. It would be nice to see REST supported in the stable 7.x-3.0 release of Services.

CommentFileSizeAuthor
#1 inputstream-1011936-d7-port.diff5.13 KBskwashd

Comments

skwashd’s picture

skwashd commented
Status: Active » Needs review
StatusFileSize
new inputstream-1011936-d7-port.diff5.13 KB

The attached patch allows inputstream work to work under D7. There is a few other things included in the patch:

* Updated info file with correct core version
* Added docs to DrupalStream.php
* s/private/protected in DrupalStream.php
* Made static variables instance variables as there is no static calls
* Added README.txt

My work is in a git repo on github.com, if you'd prefer to grab it from there - https://github.com/skwashd/Input-Stream. My Drupal 7 work is in the d7-port branch.

Hugo Wetterberg’s picture

Hugo Wetterberg commented
Status: Needs review » Fixed

Hi Dave,
Thank you for the patch. I already had a "port" at github (in other words a updated info file) but I've committed the docs you've written. I didn't make the change from static to instance variables as it would break the benefit of inputstream: that the input stream can be opened multiple times. This change would result in all opened "drupal://input" streams but the first to be empty.

Great work with the docs and thanks for helping out.

Cheers,
Hugo

skwashd’s picture

skwashd commented

Hi Hugo,

I can now see the point for the static variables.

Protected methods and properties allow code reuse in such a way that private does not. I think it is good practice to use protected over private. Would you consider including that change?

Cheers

Dave

Hugo Wetterberg’s picture

Hugo Wetterberg commented

Well, I don't have anything against it, and I agree with you that it makes code reuse easier. But, the chances for anyone ever subclassing DrupalStream must be pretty slim, so it isn't a very pressing concern. I actually planned to include the change, but it must've slipped my mind. If you fork hugowetterberg/inputstream and make the change I'll pull it in for the next release.

skwashd’s picture

skwashd commented

It wasn't a major thing, I just noticed it had been dropped out when stuff got merged in. I have sent you a pull request on github.

Status: Fixed » Closed (fixed)

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