This modules doesn't seem to following any:
http://drupal.org/coding-standards

I had a go at running the module through coder as well as running my eye over the code just to clean up indentation and white-spacing to make the code a little more readable. The code still has major issues (in my eyes), but I think its an improvements. I have committed my cleanup to github, you can visualize some of the changes here:
http://github.com/univate/drupal-kaltura/commit/e13f7d91dac7d8f8fb17acc1...

@grobot - I noticed you had a github fork of this module with some minor enhancements that where being ignored here. That is what I have been working on, not the version that is in cvs. I hoping that this might be the bases of fixing up this module.

I could also attempt to create a patch here against what in DRUPAL-6--2

My fork is here (which you are welcome to merge the changes into your repo):
http://github.com/univate/drupal-kaltura

Comments

univate’s picture

Also from the coding standards:

Files should be formatted with \n as the line ending (Unix line endings), not \r\n (Windows line endings).

xurizaemon’s picture

@univate - totally stoked that you've picked this up and run with it.

Will be merging some of your good work into contrib CVS shortly.

univate’s picture

The only reason I didn't change the files to use the coding standards unix line endings is that will change every single line and then result in no way to trace back all my changes. So at this stage I think we are forced to stick to the windows line ending on this project. Maybe that can be a issue that gets fixed when migrating the module to D7 (ie: start from a clean slate)

xurizaemon’s picture

Agreed - unsure of how best to handle the CRLF issue; I'm keen to get it fixed in D6 as well, so maybe we continue with coding standards cleanup but avoid the CRLF until we are ready to release 6.x-2.0, and apply that fix as the last checkin before 2.0 release. Can you see any issues with that approach?

xurizaemon’s picture

Issue tags: -Coding standards

.

xurizaemon’s picture

Issue tags: +Coding standards

.

xurizaemon’s picture

Status: Needs review » Needs work
Issue tags: +Coding standards

It breaks my heart to set this back to needs review just because I was too timid and stupid to apply a perfectly good coding standards patch months ago and now it probably only half applies. But I'm going to do it because it's the bitter truth.

ggevalt’s picture

As a non coder but very interested user, am I to assume that most of the work being done is on 6.x.2 and that 6.1.5 is really just going to be, in essence, replaced? Since our major disaster with Kaltura module yesterday, I've had a chance to read up on all the other issues still ongoing with this module and it seems more work and attention is being placed here than on 6x.1.5. Is that so?

Because even as a non coder, per se, I would say that the basic assessment of this thread is correct -- we have seen some erratic things with this module that we've never seen in any other module. And we know that it is a very complex module...

One of the things that I am wondering about is whether focus could be made on simplifying it and on working primarily with the CCK integration which seems much handier. What seems to be a major issue in 1.5 (and I am unsure whether it's fixed here) is that when there is NO media uploaded on a piece of content on which there is a kaltura video field, it seems ot send the database and Apache into dithers which is not, I realize, a technical term but best described my mood last night...

I applaud your work and am very appreciative of ongoing efforts to make this module work, conform to good coding standards and work properly. If you need any help with user testing, we'd be glad to do so.
g