Code Review -- Rdio Plugin

19 views
Skip to first unread message

jwmann

unread,
May 13, 2013, 2:32:47 PM5/13/13
to quicksilver-...@googlegroups.com
Hi there,

I've got a pretty functional plugin going so far,
Of course I'm only one person and by no means an obj-c dev

You can find the repo here: 

If you'd like to fully test the app, Rdio provides a 14-day free trial:

Fire away!

Rob McBroom

unread,
May 20, 2013, 9:42:45 AM5/20/13
to quicksilver-...@googlegroups.com
On May 13, 2013, at 2:32 PM, jwmann <m...@jameswmann.com> wrote:

I've got a pretty functional plugin going so far,
Of course I'm only one person and by no means an obj-c dev

Just from looking over the files, these are some things I noticed.

  • It doesn’t look like QSRdioPluginActionProvider and QSRdioPluginSource are used at all. You should be able to remove them, and any references to them in the plist.
  • You can remove anything ending with “Template” from the plist. It shouldn’t really hurt anything if it stays, but it will look tidier.
  • It needs an extendedDescription to explain what it does. Changing the extension from md to mdown and rebuilding with the Release config will probably take care of it. It could be a little more detailed. :-)
  • I’d change self.rdio to just rdio. In any context where the first works, the second should also.
  • What’s under QSBundleChildHandlers isn’t correct. The key on the left should always be a bundle ID, not an object type. To specify children for an object type, put it under QSObjectHandlers. You already have the same thing there, but since there are no objects of that type, you can just remove that too. I could elaborate, but I don’t really know what you want to show as children for what (if anything).
  • Related to the previous item, I think you can get rid of QSTypeDefinitions since you’re not creating anything of that type.
  • I’d get rid of the enclosing group under QSPresetAdditions since there’s only one preset (unless you plan to have more than one in the near future).

-- 
Rob McBroom
<http://www.skurfer.com/>

jwmann

unread,
Jul 29, 2013, 2:20:34 PM7/29/13
to quicksilver-...@googlegroups.com, mailin...@skurfer.com
Okay!

It's been a long while but I've finally found some time to work on this. :X
I've done everything you listed and pushed the commits.

I've figured out how to build in release mode finally and have issued an executable from GitHub

If you want to test it out, Rdio is free, as I probably mentioned above.
Reply all
Reply to author
Forward
0 new messages