Patch included: comment_page update for Drupal 6.

Beyond the obvious changes, we add a new function, comment_page_load(), which appears to be needed with the changes to the menu system.

CommentFileSizeAuthor
#11 comment_page_20080627.patch2.88 KBintchanter
comment_page_20080526.patch5.35 KBintchanter

Comments

intchanter’s picture

intchanter commented
Status: Active » Needs review
rszrama’s picture

rszrama commented

Hmm... here's a thought. I know there are a couple issues with the module as is. I'm curious if you'd mind me addressing those so I can test them on my D5 site using this module first. Then if you re-roll you patch against the fixed version, we won't have to patch in the fixes to two branches.

Thoughts?

btw, thanks for your help!

intchanter’s picture

intchanter commented

Sounds good to me, or we can get this one in and roll a new patch to bring your changes to D6 when the time comes. I'll leave the direction up to you, as I don't see it changing the amount of work much either way.

jbrauer’s picture

jbrauer commented

This is great... Looking forward to having this in D6 soon!

intchanter’s picture

intchanter commented

If you'd like to give me access, I'm willing to volunteer to maintain the D6 branch.

rszrama’s picture

rszrama commented
Status: Needs review » Needs work

I'm packaging up a 1.1 release w/ the fixes from the other issues. This involves about 4k less code as well. If you can re-roll the patch, that'd be awesome.

Also, do you have any prior CVS experience and/or are you familiar w/ Drupal coding standards? I try to make sure my modules are strictly up to par w/ the standards (although I'm not above making mistakes ; ).

intchanter’s picture

intchanter commented

While I'm not above making mistakes myself, I also try to follow the coding standards as closely as possible. I have also familiarized myself with the branching standards and hope to avoid mistakes there as well. CVS in particular hasn't had much use yet in my life, but I have experience with similar text versioning tools, so the differences shouldn't be too bad.

I'll get to work updating my patch to take the new changes into account and should have it soon.

rszrama’s picture

rszrama commented

Awesome. If you've got experience w/ other systems, that's more than I can say. : )

If you can apply for a CVS account on d.o, I'll granting you CVS access and update the page to reflect the fact that you'll be maintaining the D6 branch. When tagging it for a 1.1 release in D6 you'll just have to use the tag DRUPAL-6--1-1.

Feel free to make improvements on the code as you need for your use, though I'd encourage you to be a "benevolent dictator" and strike any feature requests that don't really fit the module. I won't plan on helping maintain the D6 branch unless there's a problem you need help resolving or I actually get a case where I can use the module on D6 and need to commit a fix or something.

Sound good?

rszrama’s picture

rszrama commented
Version: 5.x-1.0 » 5.x-1.1
AjK’s picture

AjK commented

CVS account approved.

intchanter’s picture

intchanter commented
Status: Needs work » Needs review
StatusFileSize
new comment_page_20080627.patch2.88 KB

Here's the patch updated for 5x-1.1 against HEAD. Apologies for the delay due to moving into a new place.

I've read up on branching and so on and am ready to create the DRUPAL-6--1 branch from current HEAD as soon as I have access to write to the comment_page repository.

Thanks!

rszrama’s picture

rszrama commented

You should have had access for Comment Page for a while. Go for it. : )

intchanter’s picture

intchanter commented
Assigned: Unassigned » intchanter
Status: Needs review » Fixed

Patch applied and committed. Release 6.x-1.1 has been created.

rszrama’s picture

rszrama commented

Rock on. : )

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.