This module has generated some discussion recently, and I feel really bad for the project owner. He went through a process of correcting things that a few reviewers pointed out to him. Then Joachim came along and said that he thought the module should be implemented as a Context plugin. The project owner basically said that he didn't want to do that and marked it 'needs review'. I found it after it had been sitting for another four weeks. I thought Joachim had abandoned the review, so I took a look at it and said I thought it was OK and should be approved.
Joachim disagreed, and that is what started the discussion. I asked zzolo to get involved because of the disagreement, and his conclusion was that Joachim's objection should not block approval, but he marked it 'needs work'. This confused the project owner, who basically said 'what am I supposed to do now? It also confused me, and when I asked zzolo about it, he said he had marked it 'needs work' by mistake, and that it would be OK to change it back.
Meanwhile Dave Reid has come along and said, wait a minute, is there some duplication with the Theme Key module? The project owner hasn't responded, and I think he has just walked away in frustration, and understandably so. When I looked at the project, I didn't even think about module duplication, because I assumed that would have been looked at early in the process. I looked into it, and as it turns out, there may be some overlap in functionality between the two, but it seems really wrong to me for somebody to come along at the very end of the process and say this doesn't pass muster because of module duplication. There may be an issue of overall utility, since the auto switching is only a small feature of Theme Key, but its all that Theme Autoswitch does. Still I think that out of fairness to this project owner, this application should be marked RTBC. I'd like to get some consensus before going ahead and changing it. I actually like the small modules that do one thing and don't load you down with code that you don't need.
Comments
Link, next steps
The application issue is here:
http://drupal.org/node/1092060
After reading through that thread, I agree it's an unfortunate situation. But I think we should probably hear back from the applicant before moving that to RTBC. It's a reasonable assumption that he left in frustration, but it's still just an assumption. If he did leave in frustration, approving the application now without first talking to him about what frustrated him seems unlikely to bring him back. If he's still willing to talk about it, we should maybe see if the ThemeKey maintainers are interested in another collaborator, and if markwittens is interested in working on a different project with a similar focus and a ton more users.
Sent Him a Link
I used his contact form and sent him a link to this discussion.
To me the system fails when a
To me the system fails when a reviewer gives a bunch of requests, and 2 week after they are done fails to check back. I'm blaming no one here because everyone has a lot going on. It just seems like we shouldn't make request of the applicant unless we are sure we can follow up on a reasonably timely basis.
'Lightweight' Alternatives are good
Module duplication is an issue, but in this case, I think the author has already answered the question in his comment #12:
I don't consider it a bad thing to offer duplication when it's a 'lightweight' version of something only available in a larger or 'heavier' module ... with a recommendation to place the appropriate references on both project pages ... my gut instinct would be to approve the app, in this case.
As the author of the module I
As the author of the module I would lie if I'd say I wasn't frustrated at all by the lengthy process of the application so far but I also understand we are all volunteers and have limited time to do what we do here. Don't worry however: I haven't left in frustration, I was just collecting energy to resume the "fight" ;-)
I left a comment in the application thread and set it back to "needs review"!
Another Twist
I asked Dave Reid to comment on this. But since he declined, and we heard from markwittens, I thought we had reached a consensus, and so I changed the status to RTBC. Soon after, Dave Reid came along and changed it back, using this as a justification:
As far as I'm concerned, this comment just shows a bad attitude, with maybe some 'protecting your turf' thrown in. I think what we need here is a discussion of what constitutes module duplication. I will give you an example of what I consider to be module duplication. I reviewed a project in which 95% of the code was a direct copy of the cck text module. The author tried to use his own data storage, and when he had a problem with it, he filed an issue against cck. Now that is module duplication.
I don't think we have that here. Did markwittens copy the code from Theme Key and just leave out the parts he didn't want? I doubt it. He created a whole admin interface for the project. Where did we get the idea that there can be no overlap in functionality in Drupal? Can we have only one toothpaste or one soap? That sounds like the old Soviet Union and we all know what happened to that.
In my opinion, the lighter weight modules also have their place. And this opinion is apparently shared by some others. I started this discussion specifically to avoid this situation. Maybe I acted too fast, or maybe there has been a failure to communicate. Either way, this situation really stinks.
Huh? When did you ask me to
Huh? When did you ask me to comment here on this issue on g.d.o? I don't ever remember being asked by you. Thanks for making me look more like an ass.
Senior Drupal Developer for Lullabot | www.davereid.net | @davereid
I don't maintain themekey and
I don't maintain themekey and I just quoted what's from a doc page (http://drupal.org/node/539608) about the application process.
Senior Drupal Developer for Lullabot | www.davereid.net | @davereid
Note I'm not against
Note I'm not against disapproval of the application. Just no reviewers actually checked to see if something already did this and I though a little more thought than "Themekey is not lightweight" was necessary, as per the application process. Since even though the code is different, there already exists a fairly simple module that does the same thing, and provides more options. Reviewers please actually do a search or two as well before investing time.
Senior Drupal Developer for Lullabot | www.davereid.net | @davereid
@Dave Reid, Look, I'm not
@Dave Reid,
Look, I'm not trying to make you look like an ass. I sent you an email from your contact form right after I posted this, which you're saying you didn't get, but you responded right away after I changed the status on Theme Autoswitch. So I have to wonder if this is politics or what? In hindsight, I should have given it some more time. I'm not blaming you for what happened.
There is an issue of fairness here beyond any issue of duplication or overall utility. I already said I assumed that somebody had already looked at that issue, because that should be done at the beginning of the process before you invest a lot of time in it. We seem to agree on that. You are coming from a place of wanting to do a thorough job, and I respect that, but it just isn't right to put somebody through a review process, then at the end say, sorry, module duplication, no can do. Even if there is an issue with duplication, at this point I say that the issue of fairness overrides that. If this had been pointed out early in the process, it would be a whole different story, but now it's just too late for that.
I hope you will see fit to reconsider.
One more thought
If you're going to block somebody's project, it's a lot better to just come right out and say we don't think this project does enough to warrant full project status, rather than jerk the guy around. You're dealing with people here, not computers.
Who said anyone was blocking
Who said anyone was blocking it?
Senior Drupal Developer for Lullabot | www.davereid.net | @davereid
The status was set to needs work
The status was set to 'needs work', without telling him anything that he needs to do, only that projects that duplicate and simplify will almost certainly not be approved. I'd say that effectively blocks the application.
Status is needs review
I just looked at the markwittens latest post on the application issue queue and noticed that the status is not 'needs work', but 'needs review'. I looked at security, coding style, documentation and understanding of Drupal API's (see comment #14). Dave Reid looked at module duplication, and others in the remote past looked at functionality. What further review needs to be done on this project?
---
I am just commenting to say that who is applying for a full git account is not allowed to change the status of the application to RTBC, in the same way who has written a patch for Drupal provided in an issue report is not allowed to change the status to RTBC.
Understood, but not the issue
Understood, but not the issue here ... this was a case of reviewers dissenting on whether it was ready for RTBC, while the submitter sat idly by and watched the debate in frustration.
In the end, the applicant has marked his own application as 'closed (won't fix)', and given up on the entire process ... while I understand the desire to reduce module duplication, modularity and flexibility are also key goals of the Drupal community - in this case, I still think we got it wrong; and did the applicant (and community) a disservice in the process.
markwittens did not change the status to RTBC
I did. It sounds like you are implying that I am markwittens using another account. I assure you that is not the case. I started doing reviews because I wanted to help out after reading 'Code Review Process Will Kill Drupal'. I'm beginning to understand what people have been complaining about.