This module adds support for the TV Brussel video sharing service.

To use this module, you'll first need to install Embedded Video Field, which is packaged with Embedded Media Field (from http://drupal.org/project/emfield).

Set up a content type to use a Third Party Video field as you normally would with emfield. Also ensure that you have enabled the new TV Brussel provider from the Admin screen at /admin/content/emfield.

Only Embed Code (and not the URL) from the TV Brussel website will be parsed and displayed, alongside with the thumbnail.

http://drupal.org/sandbox/hullabaloo/1170610

CommentFileSizeAuthor
#6 media_tvbrussel-1170758-6.patch13.42 KBtim.plunkett

Comments

ccardea’s picture

Status: Needs review » Needs work

This appears to be a corrrectly coded provider for the emvideo module. Note that I did not attempt to verify the regular expressions used in the project. That is up to you to make sure they are all working correctly. I'm sure your users will let you know if thy aren't.

Module duplication: Checked for duplicate modules, no problems.

Licensing: There is a LICENSE.txt file in the repository that needs to be removed (the packaging script provides LICENSE.txt. Otherwise OK.

Drupal API's. As I said this is strictly a video provider module, it appears to follow the procedure outlined for coding a provider, otherwise there is no interaction with Drupal APIs

I did notice that the 'emvideo_tvbrussel_embedded_link($video_code) ' function returns a link to the provider site. I believe the intent of this function is to provide a link to the video on the site, not just to the site.

Also in the 'emvideo_tvbrussel_settings()' function, there is nothing in the videojug player fieldset, so this should probably be removed.

Coding standards and documentation. The documentation is fine, it appears to be copied from the emvideo module, which is OK. You have a blank line between the doc block and the function signatures. The function signature should start on the line immediately following the doc block. No big deal. Otherwise no problems noted with coding standards.

The project makes no sql queries and takes no user input, except for a checkbox on a configuration form. No security issues noted.

Other than the few minor issues noted here, I see no problems with this module. It should be good to go.

hullabaloo’s picture

Thanks Christopher for reviewing my project! I have done the modifications you suggested:
- removing the license.txt file and
- removing the function 'emvideo_tvbrussel_settings()'

The 'emvideo_tvbrussel_embedded_link($video_code)' should indeed provide a link to the video on the site, not just to the site, but the videofilename Tvbrussel is using is always different from the link to the video, therefore it's not possible to extract the link from the embedcode.

hullabaloo’s picture

Status: Needs work » Needs review
svendecabooter’s picture

Status: Needs review » Reviewed & tested by the community

Checked this project as well, and the problems pointed out by ccardea seem to be fixed.
I also tested the module and it functions as advertised.

There are still some minor issues with indentation reported by Coder while running it on minor severity level. I suppose you could easily fix those...
Marking as RTBC as this is good to go in.

Sven

hullabaloo’s picture

Thanks for checking, Sven!

tim.plunkett’s picture

Status: Reviewed & tested by the community » Fixed
StatusFileSize
new13.42 KB

Please see this patch for coding standards fixes, and bear them in mind for future code.

I've given you full project creation rights.
Thank you for your patience!

Status: Fixed » Closed (fixed)

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