The porting will be taken care of once the first beta is out. Any help is greatly appreciated. =)

CommentFileSizeAuthor
#7 simpleblogroll.module.patch1.4 KBagerson
#4 simpleblogroll_D7.zip15.51 KBagerson

Comments

filiptc’s picture

filiptc commented
Priority: Critical » Normal

Actually, a 'normal' priority will do for now. =D

agerson’s picture

agerson commented

subscribe

filiptc’s picture

filiptc commented

Yes, sorry. I've been away from drupal for a while and kind of lost track of drupal 7 lately. Please be patient for an upgrade or consider contributing with patches as the code for this module is rather short.

Thanks!

agerson’s picture

agerson commented
Status: Active » Needs review
StatusFileSize
new simpleblogroll_D7.zip15.51 KB

I have converted the module to D7. Please let me know how it looks.

It requires this patch to the SimplePie Core Module.
http://drupal.org/node/1004732#comment-4279658

filiptc’s picture

filiptc commented

Thanks!

filiptc’s picture

filiptc commented

Ok. I'll give it a test and try out Git (haven't yet) to release it as simpleblogroll-7.x.dev. If no critical issue arises, it'll be simpleblogroll-7.0. Thanks, great work!

PS: consider joining in as co-maintainter! =)

agerson’s picture

agerson commented
StatusFileSize
new simpleblogroll.module.patch1.4 KB

I did notice one problem. This patch fixes it, but I don't know if it causes any other problems.

On line 221 of the original code you have a DELETE with a LIMIT and an ORDER BY. This is tricky with the new DB API. I am not sure how to do it. Is the LIMIT necessary to protect against some edge case? For more details see the comments in the patched code.

filiptc’s picture

filiptc commented

I wrote the code a *long* time ago, but having reviewed it just now, I think the LIMIT isn't necessary...

What the logic is doing at that point is deleting (first step in updating, second would be inserting) the latest entry in the database of one feeds element. I guess I was LIMITing to 1 in case I would add functionality in a later upgrade to have several elements per feed, which hasn't been done yet (right now, any feed identified in the database by its URL should only hold *one* element).

So I guess it's safe to say that removing it is not going to affect its actual functionality. ;)

filiptc’s picture

filiptc commented

After struggling a little with Git, I finally got to commit your changes (and patch with suggested fix) as 7.x-1.x-dev and 7.x-1.0-beta1 which should be visible on the project page once d.o runs through the cycle.

filiptc’s picture

filiptc commented
Status: Needs review » Fixed

Updating status! Feel free to reopen or post new issue if you find a bug. All credit to agerson for upgrade. I will update the project page once the new versions appear in the list.

agerson’s picture

agerson commented
Status: Fixed » Closed (fixed)

A beta for D7 is now available for download on the main project page.