Media Mixcloud module is a plugin for the Media module that allows you to embed mixcloud tracks.

Get that juicy code

drupalcode: http://drupalcode.org/sandbox/adnasa/1821108.git
git: git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/adnasa/1821108.git media_mixcloud
ventral : http://ventral.org/pareview/httpgitdrupalorgsandboxadnasa1821108git
sandbox page: http://drupal.org/sandbox/adnasa/1821108

Function spec.

– You copy/paste the mixcloud track url ( eg. http://www.mixcloud.com/DeNiaL/chillstep/ ) in the web tab in the media browser. Read more about configuration in the sandbox page.

I intend to develop this project for Drupal 7.x (and soon Drupal 8.x). You can find the sandbox project here

Reviews ( 17, February )
http://drupal.org/node/1904042#comment-7075160
http://drupal.org/node/1904782#comment-7075168
http://drupal.org/node/1911756#comment-7075108
http://drupal.org/node/1886432#comment-6999726
http://drupal.org/node/1868386#comment-6999568

CommentFileSizeAuthor
#6 media_mixcloud-review.patch3.9 KBaaron.r.carlton

Comments

riho’s picture

riho commented
Status: Needs review » Needs work

Hi and welcome,

it is strongly recommended you apply for a review bonus to speed things up.

The automated tools found a number of errors you should address first:
http://ventral.org/pareview/httpgitdrupalorgsandboxadnasa1821108git

Also please add a readme file.

adnasa’s picture

adnasa commented

Hi @riho,
Thank you for your quick-reply. I will check into this as soon as I can.

ParisLiakos’s picture

ParisLiakos commented

Just a quick scan:

README.txt

wrap everything to 80 characters

media_mixcloud.module and other files:

Add @file docblock.

inc directory should be renamed to includes

inc/themes directory should be moved to top level and renamed to theme.

MediaMixcloudStreamWrapper.inc

function getTarget($f) {

what $f is? give it a better more descriptive name.

MediaInternetMixcloudHandler.inc

same for $pp variable. rename it to something more descriptive.

$embedCode variable: should be renamed to $embed_code when it is not an object property

Overall: more documentation need on functions and classes

adnasa’s picture

adnasa commented

Did the clean-up according to pareview, implemented a new function and updated README.txt.
Will be checking up on reviewing other project applications and earn that review bonus. :-)

idflood’s picture

idflood commented

There are still some little code cleanup to do (mostly whitespace) according to http://ventral.org/pareview/httpgitdrupalorgsandboxadnasa1821108git

Another thing is that the project must be on a 7.x-1.x (or 8.x... ) branch, not master. See http://drupal.org/node/1127732

aaron.r.carlton’s picture

aaron.r.carlton commented
StatusFileSize
new media_mixcloud-review.patch3.9 KB

I'm interested in maturing this module as well. I took a quick stab at making some changes based on the Coder module's review feedback and attached a patch. Really minor spacing and commenting format issues, but let me know if there's anything else I can help with to get this module moving.

ParisLiakos’s picture

ParisLiakos commented 
   foreach ($parts as $part) {
     $media_mixcloud_url .= '/' . $part;
   }

why not use implode()?

serm’s picture

serm commented

Hi, Adnan!

Thanks for your module!

My manual review:

1) In the file there media_mixcloud.theme.inc check the server response 200, if the response is different from 200 could be worth deduce what that error message?
2) Some functions are no comments.

Best Wishes,
Andrew Podlubnyj.

adnasa’s picture

adnasa commented

hey guys,
thanks a lot for your input, I finally made it back after an autumn for roughness.
I'll get my hands dirty again, updated the 7.x-1.x branch according to your input but still needs a lot of work!

cheers.

adnasa’s picture

adnasa commented

I've done a couple of rounds of cleaning up
http://ventral.org/pareview/httpgitdrupalorgsandboxadnasa1821108git

@rootatwc: Yes, you're right. I should've used implode :-)

I'll have to say that I don't fully understand
No scope modifier specified for function "getMimeType"
Would someone mind explaining this part for me? What am I missing exactly?

As for the following, I do have a single new line character at the end of the file.

FILE: /var/www/drupal-7-pareview/pareview_temp/media_mixcloud.module
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
130 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

Tested using Media 7.x-1.x and File Entity 7.x-1-x.

The code
http://drupalcode.org/sandbox/adnasa/1821108.git

cheers!

adnasa’s picture

adnasa commented
Status: Needs work » Needs review

changing status, if you don't mind.

adnasa’s picture

adnasa commented
Issue summary: View changes

review bonus earned!

adnasa’s picture

adnasa commented
Issue tags: +PAreview: review bonus

I finally earned my review bonus, hurray!

now:
Updated to "work" with file_entity 7.x-2.x and media 7.x-2.x

problem
I've had trouble getting this thing to render during a view mode this time ;-(
Only time I get to see the player is in the media browser, which isn't beautiful at the moment.
I don't know what I am missing in implementing, so if some of you guys could give me a bit of guidance on what I'm missing/ what I need to do.

I've checked through the diff in file_entity and media and didn't get any indication of what to change in my implementations.

Any help is appreciated.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Status: Needs review » Needs work
Issue tags: -PAreview: review bonus

http://drupalcode.org/sandbox/adnasa/1821108.git/blob/refs/heads/7.x-1.x...
Typo on line 5
line 18 needs a scope modifier, line 22 needs a scope modifier and a docblock (so public static function getMimeType and public function interpolateUrl)

http://drupalcode.org/sandbox/adnasa/1821108.git/blob/refs/heads/7.x-1.x...
The variable_get here needs to be namespaced by your module, so variable_get('media_mixcloud_' . $setting, '');

Other than that, I think this is ready. Let me know when you've made those updates and I'll take another look. I'm usually in #drupal and #drupal-contribute.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented

Also, I haven't actually tested the functionality, so this may not be as ready as I thought.

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

PA robot’s picture

PA robot commented
Issue summary: View changes

updated juicy code section