Download & Extend
Issues

Rename custom_block.module to block_custom.module

Posted by sun on February 19, 2013 at 5:16am
Project:Drupal core
Version:8.x-dev
Component:block.module
Category:task
Priority:major
Assigned:larowlan
Status:needs work
Issue tags:API clean-up

Issue Summary

Follow-up to: #1871772: Convert custom blocks to content entities

Problem

  • custom* module names are the informal standard namespace for custom developed, site-specific modules.
  • I have at least a dozen of sites that have a custom_block.module already (among other custom* modules).

Proposed solution

  1. Rename custom_block.module to block_custom.module.
  2. Move the module directly into /core/modules/
  3. Add a block_custom.module component for issues to d.o.
Login or register to post comments

Comments

#1

Posted by sun on February 19, 2013 at 5:18am
Title:Rename custom_block.module to block_clustom.module» Rename custom_block.module to block_custom.module

#2

Posted by sun on February 19, 2013 at 5:54am

The module should also be moved directly into /core/modules/. Updated the issue summary accordingly.

#3

Posted by jibran on February 19, 2013 at 8:51am

+1 for this change.

#4

Posted by andypost on February 19, 2013 at 12:23pm

+1 here, this overlaps with a lot of custom modules done for specific sites.

#5

Posted by larowlan on February 21, 2013 at 8:25pm
Status:active» postponed

This will clash with #1919910: Refactor custom_block upgrade path to create tables using hook_schema_0() instead of in block_update_N() - can we get that in first?
Needs review.

#6

Posted by Dave Reid on February 28, 2013 at 3:29am

Oh god this just completely bit me for several hours. Adding a custom module completely hoses your Drupal 8 site the moment you enable it: #1929720: How the bleepity bleep do you debug missing use statements in classes? because custom_block_load() then becomes a hook_block_load() implementation with block entity objects being passed into it rather than entity IDs like it expects. And core *should* have failed gracefully if you pass in non-numeric entity IDs, except it doesn't. It explodes into a firey death from which you cannot recover due to #1003788: PDOException:Invalid text representation when attempting to load an entity with a string or non-scalar ID.

A death of a thousand suns should be administered to custom_block namespace.

#7

Posted by EclipseGc on March 6, 2013 at 4:59pm

I'd like to suggest we rename this block_content instead of block_custom. Any issues with that?

Eclipse

#8

Posted by sun on March 6, 2013 at 5:11pm
Status:postponed» active

Perhaps it's bikeshed time, even? :)

I actually wonder whether we couldn't find a name that doesn't have "block" in it… That is, because all of the function names and class names (à la CustomBlockBlock or custom_block_block_view_alter() :P) are a little bit confusing.

We apparently had a very nice and short name for certain blocks in D6 and below: Boxes.

box.module?

#9

Posted by hass on March 6, 2013 at 6:17pm

I really had some hate with block_box_get()... Very inconsistent naming and once an object and once an array and sometimes a different array structure... :-(

#10

Posted by tim.plunkett on March 7, 2013 at 9:46pm

#1919910: Refactor custom_block upgrade path to create tables using hook_schema_0() instead of in block_update_N() is in, I'm just curious to see what breaks with a relatively dumb script.

AttachmentSizeStatusTest resultOperations
custom-block-block-custom.patch83.72 KBIdleFAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.View details | Re-test

#11

Posted by tim.plunkett on March 7, 2013 at 9:47pm
Status:active» needs review

#12

Posted by System Message on March 7, 2013 at 9:49pm
Status:needs review» needs work

The last submitted patch, custom-block-block-custom.patch, failed testing.

#13

Posted by larowlan on March 7, 2013 at 9:49pm
Status:needs work» needs review

<paint target="bikeshed">how about content_block?</paint>

#14

Posted by System Message on March 7, 2013 at 10:05pm
Status:needs review» needs work

The last submitted patch, custom-block-block-custom.patch, failed testing.

#15

Posted by tim.plunkett on March 7, 2013 at 10:17pm
Status:needs work» needs review

Whoops, forgot the rename bit.

The good thing is, as long as the patch applies, picking a new name is easy, since you can just edit the patch file directly.

AttachmentSizeStatusTest resultOperations
custom-block-1920862-15.patch91.64 KBIdleFAILED: [[SimpleTest]]: [MySQL] 51,257 pass(es), 33 fail(s), and 4,834 exception(s).View details | Re-test

#16

Posted by System Message on March 7, 2013 at 11:31pm
Status:needs review» needs work

The last submitted patch, custom-block-1920862-15.patch, failed testing.

#17

Posted by larowlan on March 7, 2013 at 11:40pm
Assigned to:Anonymous» larowlan

I'll kick this along later today, I was expecting the upgrade path to fail, thats why we were waiting on #1919910: Refactor custom_block upgrade path to create tables using hook_schema_0() instead of in block_update_N() - the rest looks straight forwardish

#18

Posted by Dave Reid on March 8, 2013 at 12:31am

Not content_block, because this would result in the same problem when a content.module exists and is enabled on the site.

#19

Posted by EclipseGc on March 8, 2013 at 1:48am

Right, I still really like block_content.module. I think it actually says what it is, and solves the other problems that lead to this issue.

Eclipse

#20

Posted by larowlan on March 8, 2013 at 2:39am

block_content works for me

#21

Posted by jibran on March 8, 2013 at 6:32am

+1 for block_content

#22

Posted by larowlan on March 8, 2013 at 8:23am
Status:needs work» needs review

So this uses block_content.
Upgrade path should pass this time because block_content != existing {block_custom} table in D7.
So change to any other name should be straight forward, using block_custom will require new upgrade path that adds fields to the existing table.

Note module is still called 'Custom Block' and so are entities, I don't think 'Block Content' fits in that regards. I still like 'Add custom block' better than 'Add block content'.

AttachmentSizeStatusTest resultOperations
custom-block-rename.22.patch98.21 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch custom-block-rename.22.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test

#23

Posted by System Message on March 8, 2013 at 9:11am
Status:needs review» needs work

The last submitted patch, custom-block-rename.22.patch, failed testing.

#24

Posted by tim.plunkett on March 8, 2013 at 6:07pm
Status:needs work» needs review

#22: custom-block-rename.22.patch queued for re-testing.

#25

Posted by larowlan on March 8, 2013 at 10:47pm

green :)
We just need to agree on whether we change the entity names, module name etc too

#26

Posted by andypost on March 9, 2013 at 12:25am

@larowlan please use -M25% to minimize a patch size
I think classes and other parts should be renamed too

#27

Posted by andypost on April 7, 2013 at 11:19pm

#22: custom-block-rename.22.patch queued for re-testing.

#28

Posted by System Message on April 7, 2013 at 11:26pm
Status:needs review» needs work

The last submitted patch, custom-block-rename.22.patch, failed testing.