Youtube Player

libraries folder: appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

Hi,

Automated review (Please keep in mind that this is primarily a high level check that does not replace but, after all, eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.)

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Review of the master branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards):
  

  Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
  
  sites/all/modules/pareview_temp/test_candidate/youtube_player.module:
   +10: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
   +27: [minor] Comment should be read "Implements hook_foo()."
   +36: [normal] String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
  (...most entries removed; see attachment...)
   +302: [normal] Menu item titles and descriptions should NOT be enclosed within t().
   +312: [minor] indent secondary line of comment one space 
   +312: [minor] Format should be * Implements hook_foo().
   +313: [minor] indent secondary line of comment one space 
  
  Status Messages:
   Coder found 1 projects, 1 files, 43 normal warnings, 4 minor warnings, 0 warnings were flagged to be ignored
  

• Lines in README.txt should not exceed 80 characters, see the guidelines for in-project documentation.
• Remove LICENSE.txt, it will be added by drupal.org packaging automatically.
• @file doc block is missing in the module file, see http://drupal.org/node/1354#files .
• Comments: there should be a space after "//", see http://drupal.org/node/1354#inline
  

  youtube_player.module:33:  //TODO: dynamic forms
  youtube_player.module:214://validation block configuration
  youtube_player.module:228://numeric fields
  youtube_player.module:260:  //youtube fields
  

• Comments should be on a separate line before the code line, see http://drupal.org/node/1354#inline
  

  ./youtube_player.module:14:} // function css_block_editor_help
  ./youtube_player.module:23:} // function youtube_player_perm()
  ./youtube_player.module:212:} // function youtube_player_block
  

• ./youtube_player.install: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
  

  
  function youtube_player_install() {
  --
  
  function youtube_player_uninstall() {
  

• ./includes/views/mobile.inc: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
  

   
  function printplaylistparam' . $multi_id . '(playlistIndex,videoids,videoindexes){
  

• ./includes/utils/playlist.inc: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
  

  
  function printvideoarray($playlistlistfeed, $playlistid, $yt, $multi_id, $playlistmaxvideos) {
  

• ./youtube_player.module: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
  

  
  function youtube_player_help($path, $arg) {
  --
  
  function _youtube_player_validate($form, &$form_state) {
  --
  
  function youtube_player_admin() {
  --
  
  function youtube_player_menu() {
  --
  
  function create_content($block, $multi_id) {
  

• ./includes/utils/playlist.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function getplaylistvideos($playlistlistfeed, $playlistid, $yt, $playlistmaxvideos) {
  function getfirstvideoid($playlistlistfeed, $playlistid, $yt) {
  function printvideoarray($playlistlistfeed, $playlistid, $yt, $multi_id, $playlistmaxvideos) {
  function printplaylistlistfeed($playlistlistfeed, $playlistid, $showplaylistcontents, $ismobiledevice, $isjcarouselenabled, $yt, $maxtitlelength, $multi_id, $playlistmaxvideos) {
  function printplaylistlistentry($playlistid, $playlistlistentry, $showplaylistcontents = FALSE, $ismobiledevice, $isjcarouselenabled, $yt, $maxtitlelength, $multi_id, $playlistmaxvideos) {
  function printvideoentry($playlistid, $videoentry, $ismobiledevice, $isjcarouselenabled, $maxtitlelength, $multi_id) {
  function formatduration($seconds) {
  function formatvideotitle($title, $maxlength) {
  

• ./youtube_player.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function create_content($block, $multi_id) {
  

• ./includes/utils/Mobile_Detect.inc: The description on the line after the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  60-	 */
  74-	 */
  

• ./youtube_player.module: The description on the line after the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  20- */
  31- */
  

• There should be a space before and after operators like ==, ===, && and ||. See http://drupal.org/node/318#controlstruct
  

  includes/utils/playlist.inc:13:       if ($playlistid==$playlistlistentry->playlistId->text) {
  includes/utils/playlist.inc:63:    if ($playlistid==$playlistlistentry->playlistId->text) {
  

• Bad line endings were found, always use unix style terminators. See http://drupal.org/coding-standards#indenting
  

  ./youtube_player.css:                       ASCII C program text, with CRLF line terminators
  ./skins/youtube_player/skin.css:            ASCII text, with CRLF line terminators
  ./youtube_player.info:                      ASCII text, with CRLF line terminators
  includes/views/mobile.inc
  includes/views/browser.inc
  includes/utils/Mobile_Detect.inc
  includes/utils/playlist.inc
  skins/youtube_player/skin.css
  youtube_player.css
  youtube_player.info
  youtube_player.install
  youtube_player.module
  

• There should be no space after the opening "(" of an array, see http://drupal.org/node/318#array
  

  youtube_player.module:50:  '#options' => array( 0 => t('No'), 1 => t('Yes')),
  youtube_player.module:70:    '#options' => array( 0 => t('No'), 1 => t('Yes')),
  youtube_player.module:80:    '#options' => array( 0 => t('No'), 1 => t('Yes')),
  youtube_player.module:170:  '#options' => array( 0 => t('No'), 1 => t('Yes')),
  

• Assignments should have a space before and after the operator, see http://drupal.org/node/318#operators
  

  ./includes/views/mobile.inc:42:    for(i=0;i<videoindexes.length;i++){
  ./includes/views/mobile.inc:55:  for(i=0;i<videoindexes.length;i++){
  

• Classes and Interfaces should use UpperCamel naming. See http://drupal.org/node/608152
  

  ./includes/utils/Mobile_Detect.inc:7:class Mobile_Detect
  

• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

  ./youtube_player.css ./youtube_player.install ./skins/youtube_player/skin.css ./README.txt ./includes/views/mobile.inc ./includes/views/browser.inc ./includes/utils/Mobile_Detect.inc ./includes/utils/playlist.inc ./LICENSE.txt ./youtube_player.module ./youtube_player.info
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Edit: Manual review:

• You should really clarify about similar modules as duplication is a big issue on d.o. If your module offers something yet unimplemented, please first check whether you should raise an issue towards one of the existing projects. Or, if your project totally differs from the others, please clarify the differences. Don't feel discouraged; your initiative is really appreciated! Anyhow, there are many ways to contribute to Drupal :)

HTH, dave

I modify also the description of the project to explain that playlist creation is unimplemented in other similar projects.

Did I miss something? I cannot find this anywhere, at least not in the sense it is meant to be. It would _really_ be a good idea to have section like "The modules foo and bar do something similar, but the notable difference is $difference". Just switch roles into a site builder who has done a module search and needs to decide. If you find that there is only one (but nevertheless important) point missing in already existing modules, you should

• either raise an issue ("feature request") and include your proposed additions in a patch,
• or start a project that extends an existing module (as, in most cases, there are many possibilities to hook into their flow),
• or even contact the maintainers and apply as a co-maintainer. This, in many cases (and IMHPO!) is the most worthwile way in such cases.

Nevertheless it may be that all these suggestions are not appropriate here, but in that case you should _really_ do more clarification on that.

No offense!
dave

Hi,

thank you for the clarification. It would be really important to add all this information to your project page and also to the issue description here. Maybe you want to check with http://drupal.org/node/997024 for some tips regarding your project description.

Also, don't forget to set your status back to needs review.

Cheers!

define git clone

Still some formatting issues, see: http://ventral.org/pareview/httpgitdrupalorgsandboxweblitz1315814git

Sorry for the delay. Please be patient and wait for an in-depht review. As there came lots of new application through the holidays and we have only few people reviewing (I'm currently quite busy with my studies) this will take some time.

Please think about helping by reviewing other applications.

Also note that it's best practice to review applications with the oldest "last update" status first, so it's quite contraproductive to bump your issue periodically.

regards

@weblitz

I'm extremely interested in your module but I'm running on D7 I think that with the shift from D6 to D7 this should really be ported.

Also this patch for media youtube 6.x is a possible (though a bit limited) solution for playing whole youtube playlists on Drupal 6: http://drupal.org/node/675324

Updated to http://drupal.org/node/1323484#comment-5574450

@themebrain: please do not paste the full output of automated tools as this will just clutter the application issue. Better add it as attachment instead.

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 6.x-2.x branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

There're some issues are listed in attachment, please resolve them

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended here: http://drupal.org/node/1011698

Review of the 6.x-2.x branch:

• Remove all .settings files from your repository.
• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

manual review:

1. youtube_player_install(): empty function, so remove it. Same for youtube_player_init().
2. youtube_player_admin(): doc block: hook_admin() does not exist? See http://drupal.org/node/1354#forms
3. youtube_player_menu(): "do not include documentation of the parameters and return values -- this should be in the hook definition documentation, not in the hook implementation documentation." See http://drupal.org/node/1354#hookimpl
4. "'access arguments' => array('access administration pages'),": The administration menu callback should probably use "administer site configuration" - which implies the user can change something - rather than "access administration pages" which is about viewing but not changing configurations. Or you could of course provide your own permission.
5. .settings folder: this looks like an IDE configuration thing, remove that from your repository.
6. "@license http://www.opensource.org/licenses/mit-license.php The MIT License": all code on drupal.org is GPL, so remove that license hint. Or is this a library copied from some external source?

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

more detailed description