Figured I would get this started, since it's going to get done eventually...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kbahey’s picture

Priority: Critical » Normal
wayland76’s picture

Might be an idea to mention it here: http://groups.drupal.org/node/5036

kbahey’s picture

Version: master » 5.x-2.x-dev

Before the 6.0 port can happen, we need to fix the new Adsense code issue here http://drupal.org/node/195036

wayland76’s picture

Ok. I mentioned it on that other page.

kbahey’s picture

Issue http://drupal.org/node/157039 had some code for 6.x, but it needs to things:

- Validation that it works with the final release of D6. A lot has changed from July to January.
- Inclusion of the above issue (new Adsense code) before we move to D6.

carusen’s picture

Good luck guys in creating the port for Drupal 6.

tobe-1’s picture

Any idea when this module is available for 6.x ??

kbahey’s picture

See #3 and #5.

It would be nice if people jumped in and contributed patches or tested existing patches too.

Gijs’s picture

Hello,

this is a reply to #5

I tried out that code just now (see that link for the results), so that code seems to work/not break Drupal 6.1 .

now, how would I go about step 2, including that new adsense code?

Kind regards

Gijs

wayland76’s picture

The Step 2 you mention basically means that, until the issue mentioned in comment #3 is fixed, there will be no Drupal 6.x version. As for information on how to fix it, see the link in comment #3.

kbahey’s picture

As I said in the other issue, which you seem to have not read my replies in ...

1. The D6 code in the other issue is 6 months before the actual D6 was released, so it should be avoided/updated.

2. There are lots of stuff that went into the D5 version (specifically 5.x-2.x) and we can't leave that out.

3. The updated Google code is a crucial issue. We sure need that in D5 now before branching to D6, otherwise it will be too much work to go back and fix it. If people test the patches that ALREADY CREATED, then we will get this over with and people can move on to D6.

So, there you have it ... any other bright ideas?

wayland76’s picture

I was assuming that Gijs had tried the other code all forward-ported and everything. I realise now that this probably wasn't the case. My apologies.

hswong3i’s picture

Status: Active » Needs review
FileSize
15.69 KB

adsense 5.x-2.x-dev patch for D6 with coder clean up.
menu should not yet complete, $may_cache part not yet revamp with hook_init().

bug: not able to create adsense_click table, not able to access adsense admin page.
any idea and input?

mustardman’s picture

Just applied the patch to the 5.x code that was developer released a couple days ago. It loads but I don't get anything showing up in site configuration. When I run update.php I get the following error:

Failed: ALTER TABLE {adsense_clicks} ADD `path` VARCHAR(255) NOT NULL DEFAULT ''

hswong3i’s picture

FileSize
111.62 KB

Patch via DRUPAL-5--2, test with adsense.module and adsense_basic. Include coding style clean up and update with coder. Please test with complete uninstall and reinstall.

Able to work with Google Adesnse Injector (http://drupal.org/project/adsense_injector, http://drupal.org/node/223099).

hswong3i’s picture

FileSize
112.06 KB

Some minor coding style update.

kbahey’s picture

I appreciate all the effort going into this, but please remember that 5.x-2.x had some changes in it and needs to be tested for the new Adsense code.

Even if we have a 6.x branch working, it will require work to merge changes from 5.x into it.

Therefore focusing on a stable 5.x is a faster way to get 6.x out ...

Please go to the issue linked above and test the 5.x-2.x-dev tarball and make sure everything is working.

Only after that is done, we can use this as a base for 6.x.

mustardman’s picture

Noted,

Unfortunately, my site runs on v6.1 so I don't have much choice but to try whatever I can to get this working as quickly as possible.

turco’s picture

Sorry if it is a dumb question; how do we apply the patches to our codes exactly?
Thank you so much.

borfast’s picture

turco, try a google search for "drupal how to test a patch" -> http://www.google.pt/search?q=drupal+how+to+test+a+patch

You'll find what you need.

Raul

borfast’s picture

kbahey, I have been using the 2.x branch for quite some time and everything seems to be working fine.

Where else can I help in order to declare 2.x stable?

By the way, thanks for the great module! ;)

Raul

kbahey’s picture

borfast,

Thanks for the feedback. I just need two or three more people to do the same (using 5.x-2.x-dev without issues, and the old and new code works for them), then we can close the 5.x chapter and move to 6.x.

Taran’s picture

kbahey - I will begin testing 5.x-2.x-dev on a multiuser site today (Your2ndplace.com) and will give feedback as well. Since that site has multiple authors and uses the sharing, it will be a pretty solid test.

Stand by for heavy rolls in high seas. ;-)

kbahey’s picture

Thanks Taran ...

/me holds tight to the gunwale ...

dennys’s picture

Hi, I use 2.0 for more than 2 weeks, everything is ok. Please let me know if you want to do any special test.

kbahey’s picture

The testing needed is for 5.x-2.x-dev not 5.x-2.0.

Thanks.

Petra’s picture

I will test it this weekend.
Could we also solve this issue http://drupal.org/node/220770 before porting to 6.x?

jcnventura’s picture

I would prefer not to..

We can't honestly hope to solve all the problems in 5.x before the move to 6.x. That will only postpone it indefinitely.

João

kbahey’s picture

@jcventura

I am not asking to solve all issues for 5.x. We have one main specific issue 195036.

Issue 220770 is nice to have, but not essential before the 6.x port.

wayland76’s picture

Title: Port to 6.x » Port of AdSense to 6.x
groovypower’s picture

Can someone explain which patch should go with which version of the adsense code. I am getting failures on every combination I've tried. Thanks.

webchick’s picture

...

jcnventura’s picture

I am planning to fork this module and start with a version for Drupal 6.. This will then be backported later to Drupal 5, but that one is being well taken care of by the current version.

The module will be composed of:

1. Core module (basic support functions + single Adsense published ID)
2. New adsense format module (requires 1)
3. Legacy adsense format module (requires 1)
4. Revenue sharing module (requires 1 and 3)
5. Adsense for search module (requires 1, but able to use 4)

webchick’s picture

Hm. That sounds like a bad idea. :( Why not simply submit a patch to this module?

wayland76’s picture

@jcnventura: I second WebChick.

Magnity’s picture

Perhaps jcnventura could be added as a co-maintainer to this module. If that's not possible, then i'd support the branch to be honest - even if its not really Drupal.org good practice to have a duplicate.

The adsense module seems to have stalled, so to have someone that's willing to actually do some coding must be a benefit to everyone.

wayland76’s picture

Magnity -- I don't think it's stalled. We've been told (in comment #29) that once #195036: Implement new Adsense Code from Google is fixed, kbahey is willing to patch in the Drupal 6 patch. That's not "Stalled".

I think, though, that jcnventura's restructure is a good idea. The only thing I'd suggest is that if possible, it be made to use the Advertisement module ( http://drupal.org/project/ad ); no sense in duplicating work :).

I think we should seek feedback from the developers on this (especially kbahey).

jcnventura’s picture

To webchick and wayland76.. The reason I believe a fork at this moment would be a good ideia is:

a) kbahey is (correctly) preferring to have confirmation from several people before closing the issue #195036: Implement new Adsense Code from Google. However, I have contributed the code for that in March 31st. It's been almost two months and he's still waiting. The fix has been in the dev version for that long and no one has complained yet, so I believe that it should be closed. It is stalled for all intents and purposes.
b) I have (later) contributed code to handle text referrals and that hasn't been committed yet. This second patch may delay things further.
c) The introduction by Google of the new format code requires a vast change in this module.. Groups and channels no longer should be attached to the core module, as they aren't used by the new code at all. This profound change is easier met with a refactoring of the code than with a simple upgrade. The current code handles both formats in the same code, but the code for the old format is too complex and shouldn't even be loaded by Drupal if the site doesn't use it.
d) Since a) is blocking an 'official' Drupal 6 version, and many people (at least me) are blocked in their upgrades to D6 because of this module, I think that a workable D6 version is becoming urgent.

The plan is always (at some future point) to merge the fork's code over the current version. For that to happen of course, the D6 version will need to be stable enough, that a D5 backport can be made of it.

Last, about the ad module.. It doesn't have a D6 version yet, and since the plan is to start with a D6-only module, it can't be used yet.. I guess that when things are stable AND the ad module creates a D6 version, something may be worked out to add Adsense as an ad source to it.

João

Darren Oh’s picture

That's not a good reason to clutter Drupal.org with another project. The normal practice in such situations is to attach your version of the module to the issue for the benefit of those who don't know how to patch.

I have waited much longer for some of my patches to be applied. Working with the Drupal community to solve problems has always turned out to be a better solution than forking.

kbahey’s picture

The module has not stalled. I am using it (latest stable version of D5) on my own sites.

I am in no hurry to upgrade my sites to D6 yet, because of other dependencies.

@jcnventura

I agree that the old and new code from Google has complicated the module's code. I don't like the increased number of arguments passed in functions for many levels. However, we need to find a way to migrate form old code to new code for sites using the old code.

Instead of forking, which is a bad thing, how about this plan:

1. You submit your proposed redesign of the module as patches (for D6)? In a new issue of course.

2. Darren Oh is a co-maintainer with CVS access. Others using this module can weigh in with testing this patches. Since this is D6, they will get reviewed quickly.

3. Once we have a period of establishing trust (common practice in open source), you can be a co-maintainer too, once we see your work.

4. We can revisit D5 later, and see if we just let it die slowly, or port your D6 stuff back to D5 as 5.x-3.x. No need to commit to this right now.

Sounds Ok? What do others think?

Magnity’s picture

Firstly, it seems that my use of the word "stall" has been a bit strong for some of you. I don't mean to cause offence to anyone that's been working on the module - simply to point out as @jcnventura said in his post that progress is going quite slowly in some respects.

What @kbahey suggests seems a logical way forward. Lets hope that by having a D6 adsense module available, the other modules will also be encouraged to follow.

jcnventura’s picture

It's a good plan..

I am halfway through the re-write already.. I think that I can have an (early) preview with new format code only soon and a few days later, I will be able to add the old format + revenue sharing..

As I said, I am writing from scratch, with some cut and pastes where I feel that the current code is useful, so I will probably submit it as a standalone tar.gz.. A patch is useful for small changes, not for something like this.

João

kbahey’s picture

I opened a new issue for this here http://drupal.org/node/265666. Please attach your code there and ask for reviews.

Attaching a tar.gz is fine, and will even make it easier for people to test it (those who do not know how to patch).

Revenue sharing is more important than old code.

We can even skip the old code if there is a good migration path (i.e. in .install we have an _update_x() function that would take the old variables from a working site and convert them to the new settings). This will make for less code in the future, less legacy simpler maintenance too.

Deciphered’s picture

FileSize
112.82 KB

Re-rolled patch for latest DRUPAL-5--2

I realise there are plans to re-write the module, but it doesn't hurt to have a quick functional upgrade in the mean time.

Deciphered’s picture

FileSize
112.86 KB

Updated patch:
- 2 minor bugs fixed, basic adsense tested.

MTecknology’s picture

This is probably really stupid and maybe I didn't get this from previous posts.

Is there any really simple way for us to get a copy of the most recent code for the 6.x version?

I realize there will be many codes, I realize there will be a lot of patches where I may also need to completely drop what I already have. I'm ok with that. I just want something.

Thanks,
-Mike

mcurry’s picture

Subscribing

yas’s picture

subscribing

dfallin’s picture

subscribing

PGiro’s picture

subscribe

netentropy’s picture

if you use Google's new code won't that end the revenue sharing part of the adsense module?

kbahey’s picture

Status: Needs review » Closed (duplicate)

Not necessarily. It means more configuration is needed (i.e. each user will have to enter their own slot id for each ad block, and we store that in a table.

See here for the discussion http://drupal.org/node/265666

Deciphered’s picture

Status: Closed (duplicate) » Needs review

This is not a duplicate, it is an alternative option until the refactor is complete.
I need to use AdSense now and this version allows me to.

kbahey’s picture

If it allows you to do what you want, then that is perfectly fine, for your own purposes.

Remember, migration from this to the new code base will have to be considered.

The maintainers of the Adsense module will NOT maintain too branches. So it is better if we all pooled our resources and focused on the new code base, and discover any bugs or even architecture issues as early as possible.

Deciphered’s picture

I completly understand that this particular branch won't be maintained, and while I could have kept the patch to myself I'm sure there will be some people out there as impatient as myself that want to use Adsense for the time being.

I will also look at the refactored version and do what I can for that as we do need to get d6 modules rolling out.

kbahey’s picture

Status: Needs review » Closed (duplicate)

Thanks for sharing the patch.

What I did not want to happen is that some would be under the illusion that the old code will be maintained, and they face upgrade headaches.

If you (and anyone else reading this) can help with any aspect of the refactoring, please do so.

Thank you for your understanding and efforts.

Marking this as duplicate again, so that people don't get unintentionally misled by this issue.

jcnventura’s picture

To everyone subscribed to this thread, please get the latest version from http://drupal.org/node/265666#comment-922848 and test it in your sites.

The faster we can confirm that we have a usable D6 version, the faster we can release v1.0.

netentropy’s picture

If you upgrade to the new adsesne code, can't users doing revenue share import the code into their adsense account and change it?

jcnventura’s picture

I couldn't understand the question very clearly.

If what you're asking is if it is possible to port the revenue sharing modules to the new version, then the answer is yes.

It should be simple to take Deciphered's patch (in #45) and add support to the patched revenue sharing modules. The major change would possibly be to the settings area.

If someone makes it work, it would be appreciated.

Also, please stop posting to this thread. Use the main thread.

armanschwarz’s picture

Dunno if this is the right place, but I've built an advertising module that can be used with many types of advertisers with Drupal 6.x. At the moment I've only created the Adsense advertiser template for testing. This development version only allows users to ad their own ads (ie. no revenue sharing yet) on blog or forum pages that they have created. It's not much but it's a start so please check it out:

http://www.bottleweb.org/sites/default/files/ads%20dev%206.x-1.0.rar

Let me know what you think, here's the forum: http://www.bottleweb.org/internet/drupal-development/ads-module-drupal-6-x

There's a test user account you can use to try it out on bottleweb.org as well. I'm really glad that modules like Adsense and Advertisers exist, but I think it's time a project was started on building a complete advertising solution for Drupal, for any advertiser and any drupal setup. That's where I'd like this module to go, and I've done most of the work by building the basic code, the rest is just a matter of interfacing with users/administrators and getting the sorts of features that people actually want (for that I need feedback).

kbahey’s picture

No, it is not the right place, and as jcnventura said above: "please stop posting to this thread."

armanschwarz’s picture

My bad. The main page was closed as well so I didn't really know where else to mention it.