Project Applications which require Core edits - Is this a blocker?

Events happening in the community are now at Drupal community events on www.drupal.org.
Posted by jthorson on July 30, 2011 at 7:39pm

Interesting debate going on in http://drupal.org/node/1162758 ...

The applicant has submitted a module which requires a core edit ... the core edit basically adds a new hook to core, which the module then takes advantage to implement its functionality.

In discussion, the applicant makes the following point:

So, why don't allow people to make a choice themselves? Publish the module and we will see whether it is convenient to add 1 line of code to the core or not.
There are some modules out there that required lot more of core modifications.

What are people's thoughts/opinions on this? Does requiring a core edit 'block' an application?

Comments

Yes, blocker

Posted by sreynen on July 30, 2011 at 8:45pm

I don't think we can let projects through that encourage end users to hack core. While his particular edit looks harmless enough, it's just a terrible habit to get into for several reasons. I left a comment in the thread saying this, and also encouraging the maintainer to take advantage of Drupal 7's increased extensibility, where the core hack is no longer useful.

Depends: change should be small and in a patch

Posted by greggles on August 1, 2011 at 5:40pm

I think it's OK to include a patch to core and instructions on how to apply it if the change is minimal/reasoanble and there is an issue in development branch of core to fix the issue.

Both the Pathauto module (in the 4.6 days) and the Comment Notify module (in the 4.7 and 5.x and even 6.x days) shipped with suggested changes to core. By the time I took over Pathauto the change was in core and not needed. For comment notify I switched from shipping a replacing comment.module to shipping a patch and ultimately there was a fix into core that meant the patch was not necessary.

Given the precedent that it can be necessary I think it's not a blocker on its own.

Its conditional, but possible

Posted by zzolo on August 18, 2011 at 4:52am

SimpleTest for D6 requires a patch to core. Yeah I support if there is a solid issue around it, and there are clear instructions, and implications about the change.

I have not read the code, but definitely, if it can be done with hooks, then it should be, there's really no excuse for that.

--
zzolo

Code review for security advisory coverage applications

Group organizers

Group notifications

This group offers an RSS feed. Or subscribe to these personalized, sitewide feeds:

AltStyle によって変換されたページ (->オリジナル) /