Hello hingo,

I am reopening HEAD in order to start working on the D6 port of the module:
http://drupal.org/cvs?commit=92004
Drupal 6 RC1 is due out within the next few days.

In practical terms, this means that from now on, any patch or fix you apply to the DRUPAL-5 branch must also be ported to HEAD.

Comments

beginner’s picture

beginner commented

Well... I wanted to upgrade this module first, because it is the simplest and I wanted to use it to get a first taste of Drupal 6.

The upgrade was so easy... too easy: there is not a single code change for this module!

Actually, there should have been at least 1 change, but because of a bug in the Drupal 5 version of footnotes, I didn't have to do anything.

The only change that affected your module is the hook_init() and hook_boot(): http://drupal.org/node/114774#hook-boot-init
In Drupal 5, the call drupal_add_css() should have been put in hook_menu() // $may_cache == FALSE, but you put it in hook_init() instead.
It turns out that hook_init() has become the right place to put drupal_add_css() in Drupal 6.

Thus, the only thing I would have had to change didn't need to be changed after all.

You might want to correct this in Drupal 5: the difference is only one of performance: the css can be cached together with the page. Drupal_add_css doesn't need to be called at every single page request.

Anyway, since the module is D6 ready, I'll tag it and create an official release.

beginner’s picture

beginner commented
Version: 5.x-1.x-dev » 6.x-1.0
Status: Active » Fixed

@hingo.

Here are some more important notes.

I both created a DRUPAL-6--1 branch and a DRUPAL-6--1-0 tag for the module. This is to keep up with the new standards that are being promoted at drupal.org
See
http://drupal.org/handbook/cvs
http://drupal.org/handbook/cvs/quickstart

This means that we now have 3 working branches:
DRUPAL-5
DRUPAL-6--1 (note the new branching format).
and HEAD.

Bugfixes should all go to DRUPAL-6--1 and HEAD.
New features (if there are any!) should all go to HEAD only. If necessary, we can later make a second release based on the branch DRUPAL-6--2.

Since the code base is exactly the same (!!) it shouldn't be too much work to apply the same patch to 2 or 3 branches.

If you are unclear about the new policies, check the handbook. You can ask me, too :)

beginner’s picture

beginner commented

See:
http://drupal.org/node/68305/release

beginner’s picture

beginner commented

Check this page, too:
http://drupal.org/node/68305/edit/releases
Do you have something for 6.x in the column "Current default release" ?

hingo’s picture

hingo commented

In Drupal 5, the call drupal_add_css() should have been put in hook_menu() // $may_cache == FALSE, but you put it in hook_init() instead.
It turns out that hook_init() has become the right place to put drupal_add_css() in Drupal 6.

1) Thanks for starting this. I guess you are the main Footnotes pundit responsible for kicking the codebase towards every new Drupal release.
2) Actually, the drupal_add_css() was in hook_menu() until a few weeks ago. I changed it to make it D6 compatible. Sorry for not announcing this in any way (it is in CVS log though). Yes, I already have been trying out D6 myself.
-> Now that you mention it, I didn't test this in D5 anymore? It seems this will work on D5 but effectively disable caching: http://api.drupal.org/api/function/hook_init/5

Seems like I just created a bug on D5 then...

hingo’s picture

hingo commented
Status: Fixed » Active

Check this page, too:
http://drupal.org/node/68305/edit/releases
Do you have something for 6.x in the column "Current default release" ?

I have nothing. As I wrote above, I did the small changes needed directly to the D5 development branch. In addition to the hook_init() issue I had to add a line to footnotes.info. D6 requires modules to explicitly state which core version they are compatible with.

**
I see you already created the 6.0-1.0 release, but why isn't it showing as "current default release" yet?

(I'm changing this back to active just to get an answer on this last question.)

beginner’s picture

beginner commented

I was wondering the same thing, so I don't know.
I have seen reports by other maintainers complaining of the same thing. Maybe I missed a not so obvious step, somewhere.

I don't have much time to investigate this today. We might find the solution by searching the Drupal infrastructure / web site issue queue.
There is no hurry, though. D6 is not yet out, and the module is clearly ready :)

I saw the added line in footnotes.info (compatible = 6.x). I was surprised to see it there and I thought the CVS script added it automatically.

You shouldn't have used the DRUPAL-5 branch for D6 development.

Be aware that we now have 3 open branches, so don't get confused!

Unless you find the solution before me, I'll follow up on this issue another day.

hingo’s picture

hingo commented

Ok, if there are others asking the same thing, we can just let it rest for a while.

***

About the branches, I got the impression that the changes needed for 6.x could have been done without any effect on 5.x (in particular, D5 doesn't mind the line "compatible = 6.x" being present) so I thought it was a clever thing to do to incorporate the changes to 5.x in order to have a single codebase. Seems like it wasn't that clever after all.

Note to others: I already created a separate bug for the 5.x drupal_add_css() issue. http://drupal.org/node/202371

beginner’s picture

beginner commented

I keep an eye on this issue (official D6 release), since I am porting a few modules.

as to the branches... it's just a matter of being organized... especially when there are several of us working on the same code base.

Happy holidays. Have fun! :)

I'll check back here before D6 is out.

hingo’s picture

hingo commented
Status: Active » Closed (fixed)

It seems the default releases page is working now.

I just made releases 5.x-1.3 and 6.x-1.1 to fix http://drupal.org/node/166628

Unless surprising bugs turn up, this should be the last release of 5.x and new features now only go for 6.x.