Missing break condition on fivestar_comment_comment('insert')

In an effort to fix #211517: Prevent a user from rating through multiple comments with hook_votingapi_delete(), I found that this bug caused the fivestar widgets not to show up on the comments at all because they were getting deleted by my hook twice.

I came to the same conclusion, so +1 from me. I haven't done extensive testing on it, so I don't want to say RTBC.

+1

The 6.x branch needs some love. I have touch any 6.x code but I'll start here.

We can no longer support the Drupal 6 version of Fivestar. It is in security maintenance mode only. When the Drupal 8 version of Fivestar is released, the Drupal 6 version will be officially deprecated.

I updated the project page this morning:

"Fivestar for Drupal 6, 7, and 8

Fivestar currently works best with the latest stable release of Drupal 7. The Drupal 7 version of Fivestar is actively maintained, and will be until the Drupal 9 version is released. Please make sure you are running the latest copy of Drupal 7 before using Fivestar.

We will soon start the work to port Fivestar to Drupal 8, with the intention of it being ready when Drupal 8 is released - or shortly after.

Consequently, we can no longer support the Drupal 6 version of Fivestar. It is in security maintenance mode only. When the Drupal 8 version of Fivestar is released, the Drupal 6 version will be officially deprecated."

I'm the only active maintainer - if you don't believe me, check out the dates of commits over the last 18 months. I came late to the Drupal party, and have only worked on D7. Since getting commit access I've got the D7 version of Fivestar into a state where it's just about ready to create a clean release. I thought it would be more honest and transparent to state the truth on the project page rather than pretend that the D6 version is being actively maintained.

The D6 code is completely untouched by me, so I don't understand your comment " If you are unable to leave the last available D6 code in a workable state, then I don't have much confidence in you maintaining any other Five star branches."

I've spent quite a lot of time from December onwards putting TLC into Fivestar, and I'd be quite happy to drop it and do something else.

@ericduran - feel free to delete my maintainer status.

@Dave Reid - Seriously? This issue hasn't been touched for 2.5 years.

If someone wants to step up to maintain the D6 version, great. Let's re-open this issue and put out a final release.

But it's not going to be me. Probably not going to be you (you've got enough modules on your plate). Not sure if it will be Eric either.

But this does run back to how we choose to maintain modules in this community.

https://drupal.org/project/issues/search/?issue_tags=maintain

It can take years to find someone to step up to co-maintain even a popular module. It can take just one pissed off moment to loose a contributor.

We have to do better...

@Dave Reid - I do totally hear your frustration and really appreciate your offer. @ericduran is probably the best one to set you up with that role.

I think closing issues like this is better than just removing the D6 release, which other project maintainers have done: #2198015: Support D6 for as long D6 core will be supported

They are both strategies to deal with the guilt, workload, expectations of maintaining a Drupal project. Something that very few of us seem to have time to do.

I've been looking at ways of highlighting what are best practices for module maintenance #2186377: Highlight projects that follow Best Practices

This is a bigger issue than just with Fivestar..

@mgifford - thanks for moderating/being the voice of reason.

@Dave Reid - apology accepted. In turn, I apologise to you for my action which you've found to be rude. That wasn't my intention of course. I'm deeply into getting up to speed on D7 and getting the first clean release of D7 Fivestar out. Then I have to sort out the loads of remaining D7 issues before moving on to getting it on to D8. I never used/programmed D6, and I don't think it's viable for me to learn all three Drupal versions in a short period of time. There's nobody else available to fix the outstanding D6 issues, so it only seemed fair to let people know that. If this has triggered someone to come along and help fix D6 bugs, that's great. There was never any intention of excluding anybody else from coming in and helping out with D6; indeed, by simply setting all outstanding D6 bugs as Closed( won't fix) it allows someone to come along, reset status to Active, and fix things. Just removing the D6 branch would have been more drastic.

Having looked around the project page, I found that @ericduran has given me the permission to administer maintainers. I'm sure he wouldn't object to your request, so I've made you a maintainer as you requested. If you don't feel comfortable with me doing this, please reach out to Eric to confirm that he's ok with it. Feel free to do as much work as you want on D6 (or any other version for that matter).

I hope we can patch up our differences and work together.

Philip.

I've created a support request calling for a D6 tester: Help testing Drupal 6 patches. If someone comes forward, I'll put the D6 version back to normal maintenance mode, applying fixes provided by the community. Meanwhile, @Dave Reid feel free to apply your patch and release as agreed.

Thanks @Dave! Sorry it was such a pain to do.

Do we have docs somewhere on the proper way to close a branch? I don't disagree with your approach.

I do wonder if it might be possible to break the Issue block on the project page down by version.

All issues
open 228 D6 / 200 D7, 1817 total
Bug report
open 40 D6 / 75 D7, 707 total

One of the things folks worry about when looking at a module is how many open bugs there are. If you're effectively splitting the role of maintainer between supported versions of Core (potentially) it would be useful to be able to get the number of open bugs for the D7 version down to 0. If you are just ignoring the D6 bugs then you'll never get this down to 0....

I'm not sure what effect that would have on maintainers though.

Dave, you can start with these two. I have used these patches successfully for at least 6 months and they work fine. One of them is a one liner fix. I'll bet the 2nd one is probably still an issue in D7 too as the fix would never have been ported forward.

#601160: Comment-cast ratings not deleted with comment
#2065671: Fivestar_comment: deleting a comment re-calculates node average incorrectly

@mr.j - might be best to put that in a new issue as this one is fixed.

EDIT: I added this handbook page https://drupal.org/node/2212549

I can see both sides of the argument: the need to show users and potential users the real state of the branches vs. ease of maintenance. I favour the former. Enhancing D.O. so that the project page displays statistics by Drupal versions as @mgifford suggests would very nicely solve this. @Dave Reid - if I'd known that having to manually search through every D6 issue looking for those with patches was what was bugging you, I'd have offered to do that myself and create a new task documenting them. Indeed, I will make that offer now - would such a list be helpful for you?

The other thing to consider is that nobody had shown any interest in committing those D6 patches. One of those you've just committed was available on D.O four years ago, and the other 3.5 years ago. One might say that me acting in this way has (unintentionally) triggered someone who cared to come forward and help.

Also note that there hasn't exactly been a flood of volunteers to do D6 testing since I posted my request earlier this week.

Anyway, we are where we are and there's no changing history. Thanks to both of you for your contributions.

@whiteph - I added this issue here #2212953: Enhancing Project Page Display to include Statistics by Drupal Version

Please add your support for this issue.

Re #20 - done.

Thanks!