Field YouTube

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

• The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
• Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.

Hi Turek, thanks very much for this module! I find it useful for exactly the same reasons as you wrote. It works and does the job well! It would be great to integrate it with Insert module, so youtube fields could be inserted in the body itself. Unfortunately, I don't know how to do it, as I'm not a developer, but I might try to find out what is necessary for the integration.

However only the module from the zip file works for me. If I go to the repository (via development / repository viewer) of the sandbox module and download the latest snapshot and copy to modules dir and rename, it is not recognized. The only difference is the fieldyoutube.info file, where in the repository version there is just ' "name = Field YouTube" ' while in the zip version there is

; $Id$
name = Field YouTube
description = Defines an YouTube field type.
core = 7.x
package = Fields

files[] = fieldyoutube.module
files[] = fieldyoutube.install
files[] = fieldyoutube.class.inc

Your .info file is not complete. I can't tell what version of Drupal it's for. I think your Project Description in the sandbox should tell a prospective reviewer or user what the module does and other details. Can't really tell from looking now.

Regarding http://drupal.org/node/539608#motivation I would opt for Status: duplicate as the motivation of Turek is

I haven't been active for the community lately, and would like to engage more.

and

Yes there is media module and youtube support, but as I tested it, it was buggy.

@Turek: please report those bugs into either http://drupal.org/project/media or http://drupal.org/project/media_youtube projects ... they could use some help :-)

Although some uses of this this module and the media module may be duplicate, there are other uses that are not, or at least it seems to me from what I have read and tried. They seem to me as different concepts:

- media module - with media assets which can be reused elsewhere in a site, but than facing problems deleting the media with the the node deletion to which it was uploaded.
- field youtube module (or other similar modules) - which just attach something to an entity, that is not in any central repository or media library, and is automatically deleted when the node is deleted and does not appear anywhere else.

I used it to add a simple youtube field to comments, that I do not want to reuse and do not want it to cumulate anywhere - and Media module approach doesn't seem to me to be suitable for this simple case. I'm however new to Drupal and may be I'm wrong, but from my perspective this module is practical and actually making Drupal 7 more usable for me.

Since the missing embedded media field in D7 is replaced by the Media module, I was also looking for CCK YouTube replacements. I've tried the Media module just like Turek did, and came to the conclusion that it provided loads of functionality that I didn't need, and it wasn't functional due to a couple of critical bugs. I don't want to got back to D6 for this site just because of that. Thank you for this module Turek!

Providing patches and assistance with the Media module would be impossible because the bugs often have nothing to do with what you're trying to achieve, and you would have to dig into the whole machinery of the module to solve the problems - meanwhile the project deadline gets closer.

I hope I didn't scare Turek that much but his project still needs work.

Please fix the reported issues. Don't forget to set status to needs review after committed to your sandbox project.

Powered by Dreditor (triage sandbox) and Triage transitions

I really hope this moves forward, I just tried Media module for the first time and its just way too much for my users purposes - I hope this gets some momentum soon. Subscribe.

I have no idea what happened just now with the Component, I can't seem to change it back either.

Yes...me too. It has a lot of potential. Here is a proposed direction...I hope it helps.

Keep it simple. The core functionality IMHO:
- Support for various youtube url types (short urls etc)
- Expose the field to Views
- Grab the thumbnail from youtube also and expose it to Image presets

How does this compare to Video Upload?
Considering that the module is pretty much inactive at the moment (even though it has a D7 port, and some interesting solutions in the 6.x-2.x branch), might not be a bad idea to continue efforts there.
Otherwise we get even more duplication and half-baked efforts (media is always very hard for a one-man operation)...

Thanks for this module, it's exactly what I needed... the Media project is nice and very powerful, but for the purpose of the site I'm building it was confusing overkill; it doesn't get much simpler than pasting a URL in a text field.

I wonder if this could be folded into the 'Media Internet Sources' module as a new widget type; so that instead of having to use the Media browser, you could provide a plain text field to paste URLs from various services.

you can download the same module as a package from here:
http://windmill.sk/project/module/fieldyoutube

- info file name fixed
- small code style fixes

Turek, thanx - you are the savior! Pls pls do come on support this module and make a stable release if possible, thanks in advance. Long life to Field YouTube

Wow - this is great, except that the thumbnail links to the YouTube video only - I see no way to get the thumbnail to link to the node. So when you click on a thumbnail, you're taken out of the site.

When it comes to thumbnails, Media_youtube has no links on the thumbs at all, and with MediaFront, videos don't play reliably on iPhone and, at least on my system, fullscreen mode doesn't work.

Is it possible that there's yet no real solution for YouTube videos in D7? Holy moly. Well, it's not like video is an important element for websites, I guess. (I'm being sarcastic... yikes!)

This module looks like the closest and the easiest if I can only get the thumbnail to link to the node.

Thanks for the work here - thumbs up... almost!

There where a couple of typo's / minor issues that haven't been resolved in a long time on Turek's sandbox or website or zipfile.

I wish we could get a project for this so we don't have code scattered everywhere.

This is a stable, simple CCK field. Can we get this as a project yet?

Here is my sandbox for the fixes, I've requested Turek to pull: http://drupal.org/sandbox/youdontmeanmuch/1441980

I would also like to add some of the basic features described above if this where to make it as a project.

I have contacted Turek a long time ago, that I can create the project for him if he would like to maintain it.
But I would create the project for anyone if he/she is willing to maintain it.
@youdontmeanmuch: are you interested?

@Pasqualle I'm very interested in maintaining and improving this module.

hmm, sorry @youdontmeanmuch, I wanted to create a new project for you, but it turned out that almost identical module already exists http://drupal.org/project/youtube and I do not want to create a duplicate. Turek's sandbox project is 10 months older than the youtube module.

This issue is a shame of Drupal's project application process..

@youdontmeanmuch please don't give up, if you, or anyone participating in this issue, or using this module is still interested in improving it please join that youtube project. Report any missing features or bugs, and try to apply for co-maintainership status. I am sure contributors starting as co-maintainers will get 'Create Full Projects' permission faster than trying this thing called "Drupal.org Project applications"..

quick recipe how to gain co-maintainer status:
1. choose a module you like or want to know better (any module)
2. read the issue queue or find a bug
3. create 1 single patch and upload it to that issue
4. talk with the maintainer on irc, or create an issue where you ask for co-maintaner status
(5) if you do not get an answer and the project seems unmaintained follow the http://drupal.org/node/251466 "Dealing with abandoned projects" process

unfortunately, these steps work with more than 80% of Drupal modules

if you are a good co-maintainer of 2-3 modules and would like to create own modules which do not exists yet, just create a new issue in "Drupal.org Project applications" queue. Nobody will care that you do not have a sandbox project, as you already proved that you are a good Drupal contributor, you must get git access by definition.

I have no idea how I missed that project. I'll switch over to that, also appreciate the advice on contributing to Drupal.

There is another one project: http://drupal.org/project/media_youtube (Posted by aaron on January 27, 2009 at 10:31pm)
It is an YouTube field for the http://drupal.org/project/emfield project.
There is a beta for D7. You should try to collaborate with the maintainers of this project.

sorry, but those are different projects, and the project description clearly states why..

Hi Turek.

1. In your readme file it says:

1. Place the entire email directory into your Drupal sites/all/modules/
directory.

I'm not too sure what you mean by "email". I'm assuming it's a typo.

2. In step #3 you say "Add your new field in "Content Types" but I think it would be clearer if you said:

Add your new field in "Content Types"->"Manage Fields"

3. After adding that field to my default 'Basic Article' content-type I get a:
Notice: Undefined index: field_youtube_custom_settings in field_youtube_field_widget_settings_form() (line 138 of /var/www/html/drupal7/sites/all/modules/field_youtube/field_youtube.module)

That's probably due to how you're doing your variable_get in the field_youtube.module settings_form. You're doing:

$form['field_youtube_custom_settings']['field_youtube_width'] = array(
    '#type' => 'textfield',
    '#title' => t('Width of rendered video'),
    '#default_value' => variable_get('field_youtube_width', $settings['field_youtube_custom_settings']['field_youtube_width']),
    '#required' => FALSE,
  );
  $form['field_youtube_custom_settings']['field_youtube_height'] = array(
    '#type' => 'textfield',
    '#title' => t('Height of rendered video'),
    '#default_value' => variable_get('field_youtube_height', $settings['field_youtube_custom_settings']['field_youtube_height']),
    '#required' => FALSE,
  );
  $form['field_youtube_custom_settings']['field_youtube_quality'] = array(
    '#type' => 'radios',
    '#title' => t('Default video quality'),
    '#options' => array(0 => t('Normal quality'), 1 => t('HD quality')),
    '#default_value' => variable_get('field_youtube_quality', $settings['field_youtube_custom_settings']['field_youtube_quality']),
    '#required' => FALSE,
  );

when it should be something like this:

 $form['field_youtube_custom_settings']['field_youtube_width'] = array(
    '#type' => 'textfield',
    '#title' => t('Width of rendered video'),
    '#default_value' => variable_get('field_youtube_width', ''),
    '#required' => FALSE,
  );
  $form['field_youtube_custom_settings']['field_youtube_height'] = array(
    '#type' => 'textfield',
    '#title' => t('Height of rendered video'),
    '#default_value' => variable_get('field_youtube_height', ''),
    '#required' => FALSE,
  );
  $form['field_youtube_custom_settings']['field_youtube_quality'] = array(
    '#type' => 'radios',
    '#title' => t('Default video quality'),
    '#options' => array(0 => t('Normal quality'), 1 => t('HD quality')),
    '#default_value' => variable_get('field_youtube_quality', ''),
    '#required' => FALSE,
  );

You should see the documentation for variable_get. The second argument is its default value. If you're setting its default value to something that hasn't been defined yet, you'll get the same error I did.

I used the Coder module to look at your code and came up with a lot of issues. It would probably be worth it for you to install it and then go to this url:
http://whateverSiteYouInstalledYourModule.com/admin/config/development/c...

.....there are a lot of things worth noting there. A lot of your code is full of odd spacing and downright hard to read. Good news is that your 'install' file looks good though!

Finally, after installing your module and then using a 'youtube' field on my content type, I never saw the video in the rendered node. That issue could be that I'm doing this on a local box though.....I'm not too sure. I didn't dive too deeply into your code to troubleshoot the issue but, to me, it doesn't look like you're using a lot of the things I'm used to seeing in a module ('theme' functions being the main one).

Perhaps I'm missing something?

Looks like you're off to a good start though!

Sorry I still think this module has too few differences to https://drupal.org/project/youtube.
Please consider joining forces with the maintainer of the youtube field module.

If you still think it's totally different from youtube field module you can surely reopen.

I'm really sorry about the huge delay in the application process.
Feel free to come back with an other module and assign it directly to me, I'll take care of it asap!

Sorry :(

As full projects have precedence to sandbox projects it unfortunately does not matter who was first, it's a matter of whether there is a full project or not.

I think it is possible to review your module and approve you but anyway - don't make a duplicating full project out of it, rather join forces with the existing youtube field module.

If you feel okay with that we can continue on this.
As said I'm sorry for the delay but as reviewing applications is a volunteer work it's hard to make sure everybody gets trough this quick.

We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

Turek,

You said:

@AaronELBorg did you downloaded latest code, becouse I think I already fixed that?

I grabbed the zip file from your post dated: March 12, 2012 at 4:23pm. Not fixed in that one.

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:

• Remove LICENSE.txt, it will be added by drupal.org packaging automatically.
• Remove all old CVS $Id tags, they are not needed anymore.
  

  field_youtube.class.inc:2:// $Id$
  field_youtube.install:2:// $Id$
  field_youtube.module:2:// $Id$
  README.txt:1:// $Id$
  

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
• 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.

Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards

sites/all/modules/pareview_temp/./test_candidate/field_youtube.class.inc:
 +2: [minor] Commits to the Git repository do not require the CVS $Id$ keyword in each file.

sites/all/modules/pareview_temp/./test_candidate/field_youtube.install:
 +2: [minor] Commits to the Git repository do not require the CVS $Id$ keyword in each file.

Status Messages:
 Coder found 2 projects, 2 files, 2 minor warnings, 0 warnings were flagged to be ignored

FILE: ...l-7-pareview/sites/all/modules/pareview_temp/test_candidate/LICENSE.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 3 WARNING(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
 106 | WARNING | Line exceeds 80 characters; contains 81 characters
 120 | WARNING | Line exceeds 80 characters; contains 84 characters
 227 | WARNING | Line exceeds 80 characters; contains 81 characters
--------------------------------------------------------------------------------


FILE: ...al-7-pareview/sites/all/modules/pareview_temp/test_candidate/README.txt
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 37 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: .../sites/all/modules/pareview_temp/test_candidate/field_youtube.class.inc
--------------------------------------------------------------------------------
FOUND 49 ERROR(S) AND 5 WARNING(S) AFFECTING 49 LINE(S)
--------------------------------------------------------------------------------
   2 | ERROR   | You must use "/**" style comments for a file comment
   2 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
   2 | ERROR   | There must be no blank line following an inline comment
  11 | WARNING | The use of private methods or properties is strongly
     |         | discouraged, use "protected" instead
  15 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  16 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  17 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  18 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  27 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  28 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  29 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  30 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  31 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  37 | ERROR   | Inline control structures are not allowed
  43 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  44 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  45 | WARNING | Line exceeds 80 characters; contains 87 characters
  45 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  46 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  53 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  54 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  55 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  56 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  58 | ERROR   | Inline control structures are not allowed
  60 | ERROR   | Inline control structures are not allowed
  66 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  67 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  68 | WARNING | Line exceeds 80 characters; contains 98 characters
  68 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  69 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  76 | ERROR   | Inline control structures are not allowed
  83 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  84 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  85 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  86 | ERROR   | Expected 3 space(s) before asterisk; 2 found
  93 | ERROR   | Inline control structures are not allowed
  96 | ERROR   | String concat is not required here; use a single string
     |         | instead
 106 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 107 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 108 | WARNING | Line exceeds 80 characters; contains 88 characters
 108 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 109 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 115 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 116 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 117 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 118 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 125 | ERROR   | Inline control structures are not allowed
 130 | ERROR   | Whitespace found at end of line
 132 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 133 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 134 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 135 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 139 | WARNING | Line exceeds 80 characters; contains 108 characters
 154 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...eview/sites/all/modules/pareview_temp/test_candidate/field_youtube.info
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 4 | ERROR | It's only necessary to declare files[] if they declare a class or
   |       | interface.
 5 | ERROR | It's only necessary to declare files[] if they declare a class or
   |       | interface.
--------------------------------------------------------------------------------


FILE: ...ew/sites/all/modules/pareview_temp/test_candidate/field_youtube.install
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
  2 | ERROR | You must use "/**" style comments for a file comment
  2 | ERROR | Inline comments must end in full-stops, exclamation marks, or
    |       | question marks
  2 | ERROR | There must be no blank line following an inline comment
 17 | ERROR | Array indentation error, expected 8 spaces but found 9
 18 | ERROR | Array indentation error, expected 8 spaces but found 9
--------------------------------------------------------------------------------


FILE: ...iew/sites/all/modules/pareview_temp/test_candidate/field_youtube.module
--------------------------------------------------------------------------------
FOUND 11 ERROR(S) AND 1 WARNING(S) AFFECTING 8 LINE(S)
--------------------------------------------------------------------------------
   2 | ERROR   | You must use "/**" style comments for a file comment
   2 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
   2 | ERROR   | There must be no blank line following an inline comment
  39 | ERROR   | Additional blank line found at the end of doc comment
  79 | ERROR   | BREAK statements must be followed by a single blank line
  83 | ERROR   | There should be no white space after an opening "("
  83 | ERROR   | There should be no white space before a closing ")"
  87 | ERROR   | BREAK statements must be followed by a single blank line
  91 | ERROR   | There should be no white space after an opening "("
  91 | ERROR   | There should be no white space before a closing ")"
 133 | WARNING | A comma should follow the last multiline array item. Found:
     |         | FALSE
 176 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------

Source: http://ventral.org/pareview - PAReview.sh online service

@jack_tux please don't paste these ugly automated reports in when they're so long, it's better to either provide a direct link or attach a textfile - you can still edit your comment, please do so and short it
thanks

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

Link to the project page and git clone command are missing in the issue summary, please add them.

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

Automated review found no errors. Coder module had a few suggestions regarding the docs in field_youtube.class.inc. Here is a manual review:

CODE
1. README.txt: I'm not sure I understand the first instruction of your installation notes. It mentions an "email directory".

USABILITY
1. When I first install a YouTube field in a content type and input a YouTube URL and save, nothing shows up UNTIL I go to 'Manage display' for the content type and save. I don't actually have to change anything in the settings...just save them. So it seems like the field formatter isn't getting a default value when the field is added to a content type. Other than this, it works really nicely!

This is a great little module other than the usability issue above. Also, you should consider participating in the Review Bonus program in order to 'fast-track' the approval of your module as a full project.

If you don't understand something in the readme that has nothing to do with code.
Usability issues are also very minor here.

Please concentrate on the actual application requirements, see http://ventral.org/pachecklist

Switching back to needs review as only minor stuff was pointed out in the last comment

I don't think it's minor if the module doesn't do what it is supposed to do out of the box. There shouldn't be a need to go into the display settings before a field works, should there? Also, the README file is part of the checklist, although true it is not part of code. I just came across it when I was reviewing the code. If there are instructions that apparently have nothing to do with the module, as in this case, shouldn't that be addressed as it would be confusing?

The most important parts of the review are API, security checks and basic repository usage errors new developers do, these are worth pushing to needs work.

For sure it is good to point issues out you found while using the module or in the projects documentation, but if we "enforce" minor issues most applications wont ever be fixed.

patrickd: Thanks for the clarification. Will keep that in mind.

I don't think that this sandbox project needs any more reviews, as there is a full project [http://drupal.org/project/youtube] which is a duplicate of this one. And Turek said: "I won't push this module to full projects, and would try to join forces."
So just give him the damn commit access already!!! More than 2 years waiting, when it was good enough from the first moment..

As Turek has demonstrated solid maintainer responsibility, and it surely isn't his fault that someone who already had new project permissions duplicated his project, i've granted him vetted git user access.

I hope the maintainers of Youtube Field will grant him co-maintainer status there but Turek you can start with a review of the code there like you have been receiving here!

Thanks everyone for reviews, tips, and vigorous debate.

@mlncn: please use the promotion template from https://groups.drupal.org/node/184389 :

Thanks for your contribution!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.