Rickroll.module to core

skiquel - April 1, 2008 - 06:43
Project:Drupal
Version:7.x-dev
Component:other
Category:bug report
Priority:critical
Assigned:Unassigned
Status:postponed
Description

Hi.

I want to propose a new core module for 6.2. Many may say it's too late; You know the rules and so do I. For some reason I have a strong inclination that at this moment we should put the bureaucratic dribble aside. I mean, what the hell? Let's do this.

This module is never gonna give you up, never gonna let you down.

AttachmentSize
rickroll.zip3.48 KB

#1

webchick - April 1, 2008 - 06:48
Version:6.x-dev» 7.x-dev
Status:active» won't fix

Let's be serious, ok?

#2

JohnForsythe - April 1, 2008 - 07:04

Can you backport this to 5.x? I was working on something similar, but your implementation is far cleaner. A full commitment's what I'm thinking of..

#3

Liam McDermott - April 1, 2008 - 07:23

return $lyrics;

ROFL, brilliant. This gets a +1 from me.

#4

dman - April 1, 2008 - 07:32

+1 from me.

Webchick obviously doesn't know what day it is. ;-p

#5

Liam McDermott - April 1, 2008 - 08:17

Yeah, we need an 'April fools' status for.

#6

NikLP - April 1, 2008 - 12:15

Oh, it's a joke?

File this in g.d.o in the "Drupal for Evil" group, please.

#7

bjaspan - April 1, 2008 - 12:31
Status:won't fix» patch (code needs work)

The function rickroll_enable() has uneven spacing for its last line and therefore does not comply with core coding standards.

#8

peach - April 1, 2008 - 12:44

I can't believe someone actually wrote 200+ lines of code to pull this off. LOL

#9

Wim Leers - April 1, 2008 - 13:07

Woot! Rickroll in core! :) +1

#10

catch - April 1, 2008 - 13:19

RTBC

#11

keith.smith - April 1, 2008 - 13:29

Ah, 1987.

#12

Steven Merrill - April 1, 2008 - 13:32

@bjaspan You know the rules, and so do I.

#13

timmillwood - April 1, 2008 - 15:06

if($date = 1st April){
rickroll_enable()
}
else{

}

#14

Eaton - April 1, 2008 - 16:13

+1.

#15

Dries - April 1, 2008 - 16:32

*subscribe* ;-)

#16

walkah - April 1, 2008 - 16:33

+1 A+++++++++++WILL COMMIT AGAIN!!!!!1111

#17

catch - April 1, 2008 - 16:33
Status:patch (code needs work)» patch (reviewed & tested by the community)

Go on, commit it today and roll back by midnight. There's still time!!!!

#18

catch - April 1, 2008 - 16:36
Status:patch (reviewed & tested by the community)» patch (code needs work)

Oh wait. No SimpleTests?

#19

kbahey - April 1, 2008 - 16:52

+1 on the concept.

You need to run it thru coder though.

The function _rickroll_video_default has the wrong tabs, so does rickroll_block.

The trailing ?> needs to go too.

#20

boydjd - April 1, 2008 - 17:48
Component:other» base system
Category:feature request» bug report
Priority:normal» critical
Status:patch (code needs work)» patch (reviewed & tested by the community)

+1
This isn't a feature request, it's obviously a bug to not be able to rick roll in Drupal core.

#21

Grugnog2 - April 1, 2008 - 18:09
Component:base system» other

Code style is good - nicely abstracted - RTBC.

#22

Brian@brianpucc... - April 1, 2008 - 18:24

subscribing

#23

alex_b - April 1, 2008 - 19:18

+1
Love it.

#24

bboyjay - April 1, 2008 - 20:58

What the......

Did I just get rickroll'd?

#25

flobruit - April 1, 2008 - 21:08

Before we get this into core, we need to guarantee that the look will be consistent with most themes. Based on catch's comment in #10, this patch adds rickroll.css to this module:

#main {
  background: url(http://daz.com/img/00/00/00/7417.jpg) repeat;
}

This provides both aesthetic and usability improvements, and if you have firebug installed you can test it on this page with the "Blue Beach".

edit: Sorry, patch was broken. Re-rolling.

AttachmentSize
rickroll-css-241207-24b.patch9.22 KB

#26

Senpai - April 1, 2008 - 21:12
Status:patch (reviewed & tested by the community)» patch (code needs work)

I tested this patch against a 7.x sandbox, and the patch needs some work. There seem to be to many "+?" styles in the CSS code, and it causes a White and Black Screen Of Death.

I had to un-apply the patch just to get my sandbox .bak

Please re-roll this for 6 as well, once you figure out why it's causing the Bluebeach theme to break. Thanks man!

#27

csevb10 - April 1, 2008 - 21:32

We developed this little beauty of code for one of our clients, so it's a D5 version, but with all the hullabaloo, we felt like we should share with the community....you know, in case you ever get tired of hosting Rick on your website.

AttachmentSize
rr.zip6.62 KB

#28

David_Rothstein - April 1, 2008 - 21:42

Well, clearly this module is not complete until it defines some hooks and an API. Think of all the contrib modules that could be written to extend it!! The man is a lyrical genius, and hardcoding his work into only SOME parts of core -- with no option for contrib modules to add it in other places -- is certainly the wrong approach ;)

#29

kbahey - April 1, 2008 - 22:08
Status:patch (code needs work)» patch (reviewed & tested by the community)

I tested it and it works great ...

The patch removes the stray __MACOSX (sorry to Apple fanbois), fixes all code style issues, and makes the patch in proper tar.gz format.

Screenshots are attached.

I am RTBC'ing my own patch, but it is a good occasion to do so ...

AttachmentSize
rickroll2.jpg55.97 KB
rickroll1.jpg30.89 KB
rickroll-2.tar_.gz2.37 KB

#30

kbahey - April 1, 2008 - 22:21

@csevb10

Why is the code so different from the D6 version? This will make patching from one version to the other very difficult.

-1 until they are from the same code base, only different D5 and D6 APIs.

#31

rcross - April 2, 2008 - 00:13

sweet! just what i've been missing in my life - more rickroll!

#32

Dries - April 2, 2008 - 02:45
Status:patch (reviewed & tested by the community)» patch (to be ported)

Patch applied to HEAD. Still needs to be ported to Drupal 6.

#33

xamox - April 2, 2008 - 03:10

Gotta love internet meme's. Props though, at least some of us have a sense of humor.

#34

David Strauss - April 2, 2008 - 03:28

I'm actually concerned about the scalability of this change. Has anyone tested the Rickrolling concurrency levels we can expect to achieve? I mean, we don't want to bring down the site, even if Rick does bring down the house with it.

#35

kbahey - April 2, 2008 - 03:31

David

The module only works with PostgreSQL, so we can let the 5% of Drupal sites worry about it.

Moreover, there are no open socket connections server side. The suckers browsing to the /rickroll page will have the connection from their browser to Youtube directly

#36

David Strauss - April 2, 2008 - 03:40

@kbahey We can probably throttle traffic using this transport layer: http://www.ietf.org/rfc/rfc1149.txt

#37

macgirvin - April 2, 2008 - 03:51

Doh! [smacks head] That's right, it's April 1 over on that side of the world. I'm used to ignoring Drupal timestamps because they're always wrong by an hour or by GMT offset or by a day or 3 weeks or whatever.

I just wanna tell you how I'm feeling
Gotta make you understand

#38

NikLP - April 2, 2008 - 10:44

Rick Astley started out as a tea boy in the studios of Stock, Aitken and Waterman.

It just goes to show, with a bit of grit and determination, and a lot of hard work, you could be pouring a cup of tea one minute, and the next, some idiot bastard producer has made you into a one hit wonder. Thanks, Mr Waterman.

#39

Liam McDermott - April 2, 2008 - 12:07

Thanks, Mr Waterman.

Is that the 'write the theme tune, sing the theme tune' Mr Waterman?

Right people right time,
Just the wrong location,
I've got a good idea,
Just you keep me near,
I'll be so good for Drupalcoredevelopment!

#40

Summit - April 2, 2008 - 12:15

Hi,

I miss the audio hooks in the patch.
Seeing is Believing, but without hearing the fun is less!
Greetings,
Martijn
www.trekking-world.com

#41

kbahey - April 2, 2008 - 14:11
Status:patch (to be ported)» postponed

@David Strauss

Throttling the traffic will make it worse. Each Apache process will take longer to complete, and soon we overshoot MaxClients.

Anyway, we missed the window of having this in core properly yesterday.

So, @Skiquel, my plan would be to do this:

- Create a contrib project for it, commit the code, have a 5.x and 6.x version. Contrib is the incubator for innovation, and hence having it there would allow it to get more exposure, more features, and patches without having the high bar set for core.

- Create an install profile for it, so it is ready this time next year.

- Push for Acquia to create a Drupal distribution for it.

#42

ipwa - April 2, 2008 - 16:32

Very funny, FF should have a rickroll page amongst it's hidden ages.

 
 

Drupal is a registered trademark of Dries Buytaert.