Closed (fixed)
Project:
Fivestar
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
5 Oct 2011 at 21:28 UTC
Updated:
21 Feb 2014 at 21:10 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
ericduran commentedNo problem, just didn't write it. If someone wants to write it all patches are welcome.
Comment #2
ericduran commentedComment #3
restyler commentedHere is the patch:
Comment #4
restyler commented(against alpha1)
Comment #5
ericduran commented@restyler Awesome.
A couple of comments:
Those extra white spaces should be removed
We can avoid the if (module_exists(...)). I'm thinking we set up a fivestar.targets.inc
file and in there we can declare each module as a separate hook such as user_reference_fivestar_target_info() and node_reference_fivestar_target_info()
that way those options are only available when the modules exists and also every module get implemented correctly. :)
It'll also allow us to to a user_fivestar_target_info so we can target the Node Author, etc..
Thought? I think this would be better for the long run so we can start separating out the code. But besides that everything else looks Great.
Comment #6
restyler commentedI agree, the hook approach should work fine here.
Comment #7
Shadlington commented#3 works for me :)
Comment #8
Shadlington commented-Deleted-
Comment #9
ericduran commentedJust making sure test pass. It should since most of this functionality isn't tested.
Comment #10
segi commentedIt works me too. I resubmit the patch with some minor coding standard fixing. I hope it will be the part of module, because it is waiting for a while.
Comment #11
whiteph commentedThanks everyone :)