Right now we have tons of classes in files like tmgmt.entity.inc. 8.x enforces PSR-0, which means a class per file. Git is able to track file renames but it can't track if a file is split into multiple files.

Just moving all classes into their own files would allow git to handle changes in 7.x-1.x much better and move them to the correct place.

CommentFileSizeAuthor
#4 move_all_the_things-1969494-5.patch385.81 KBberdir
#4 move_all_the_things-1969494-5-interdiff.txt519 bytesberdir
#3 move_all_the_things-1969494-2.patch385.81 KBberdir
#3 move_all_the_things-1969494-2-interdiff.txt864 bytesberdir
#1 move_all_the_things-1969494-1.patch385.82 KBberdir

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: Move classes to seperate files » Move classes to separate files
Status: Active » Needs review
StatusFileSize
new move_all_the_things-1969494-1.patch385.82 KB

This is a pretty crazy patch that unfortunately kills a lot of our git history. But it either happens here or in the 8.x port :)

The main thing that I'm worried about is that one of those controller class moves would kill a site where only drush rr would be able to resurrect it. Want to test it somewhere else with just running update.php.

Also, let's see what the tests have to say.

Status: Needs review » Needs work

The last submitted patch, move_all_the_things-1969494-1.patch, failed testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new move_all_the_things-1969494-2-interdiff.txt864 bytes
new move_all_the_things-1969494-2.patch385.81 KB

Noticed one wrong thing in there.

Note that there are no code changes in this patch. git detects some weird renames in the ui controllers and that results in strange rename diffs.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new move_all_the_things-1969494-5-interdiff.txt519 bytes
new move_all_the_things-1969494-5.patch385.81 KB

Another stupid mistake.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented 
+++ b/sources/entity/tmgmt_entity.infoundefined
@@ -5,6 +5,7 @@ core = 7.x
-files[] = tmgmt_entity.test
+files[] = tmgmt_entity.source.test
+files[] = tmgmt_entity.pathauto.test

+++ b/sources/entity/ui/tmgmt_entity_ui.infoundefined
@@ -9,5 +9,6 @@ dependencies[] = views_bulk_operations
 files[] = tmgmt_entity_ui.test
+files[] = tmgmt_entity_ui.list.test

+++ b/tests/tmgmt_test.infoundefined
@@ -3,4 +3,6 @@ package = Translation Management
-files[] = tmgmt_test.plugin.inc
+files[] = tmgmt_test.plugin.source.inc
+files[] = tmgmt_test.plugin.translator.inc
+files[] = tmgmt_test.ui.translator.inc

+++ b/tmgmt.infoundefined
@@ -8,12 +8,33 @@ dependencies[] = locale
 files[] = includes/tmgmt.exception.inc
-files[] = includes/tmgmt.controller.inc
-files[] = includes/tmgmt.entity.inc
-files[] = includes/tmgmt.plugin.inc
-files[] = includes/tmgmt.ui.inc
+files[] = controller/tmgmt.controller.job.inc
+files[] = controller/tmgmt.controller.job_item.inc
+files[] = controller/tmgmt.controller.remote.inc
+files[] = controller/tmgmt.controller.translator.inc
+files[] = entity/tmgmt.entity.job.inc
+files[] = entity/tmgmt.entity.job_item.inc
+files[] = entity/tmgmt.entity.message.inc
+files[] = entity/tmgmt.entity.remote.inc
+files[] = entity/tmgmt.entity.translator.inc
+files[] = plugin/tmgmt.plugin.base.inc
+files[] = plugin/tmgmt.plugin.interface.base.inc
+files[] = plugin/tmgmt.plugin.interface.reject.inc
+files[] = plugin/tmgmt.plugin.interface.source.inc
+files[] = plugin/tmgmt.plugin.interface.translator.inc
+files[] = plugin/tmgmt.plugin.source.inc
+files[] = plugin/tmgmt.plugin.translator.inc
+files[] = plugin/tmgmt.ui.interface.source.inc
+files[] = plugin/tmgmt.ui.interface.translator.inc
+files[] = plugin/tmgmt.ui.source.inc
+files[] = plugin/tmgmt.ui.translator.inc
 files[] = includes/tmgmt.info.inc
-files[] = tmgmt.test
+files[] = tests/tmgmt.base.test
+files[] = tests/tmgmt.base.entity.test
+files[] = tests/tmgmt.crud.test
+files[] = tests/tmgmt.plugin.test
+files[] = tests/tmgmt.helper.test
+files[] = tests/tmgmt.upgrade.alpha1.test

+++ b/translators/file/tmgmt_file.infoundefined
@@ -5,5 +5,7 @@ core = 7.x
-files[] = tmgmt_file.format.inc
+files[] = tmgmt_file.format.interface.inc
+files[] = tmgmt_file.format.xliff.inc
+files[] = tmgmt_file.format.html.inc

+++ b/translators/tmgmt_local/tmgmt_local.infoundefined
@@ -5,11 +5,15 @@ core = 7.x
-files[] = includes/tmgmt_local.controller.inc
-files[] = includes/tmgmt_local.entity.inc
+files[] = controller/tmgmt_local.controller.task.inc
+files[] = controller/tmgmt_local.controller.task_item.inc
+files[] = entity/tmgmt_local.entity.task.inc
+files[] = entity/tmgmt_local.entity.task_item.inc
 files[] = includes/tmgmt_local.info.inc
 files[] = includes/tmgmt_local.plugin.inc
-files[] = includes/tmgmt_local_ui.controller.inc
+files[] = includes/tmgmt_local.plugin.ui.inc
+files[] = controller/tmgmt_local.ui_controller.task.inc
+files[] = controller/tmgmt_local.ui_controller.task_item.inc
 files[] = tmgmt_local.test

+++ b/ui/tmgmt_ui.infoundefined
@@ -7,5 +7,7 @@ dependencies[] = tmgmt
-files[] = includes/tmgmt_ui.controller.inc
+files[] = includes/tmgmt_ui.controller.job.inc
+files[] = includes/tmgmt_ui.controller.job_item.inc
+files[] = includes/tmgmt_ui.controller.translator.inc

That summarizes this issue. Removed files have been split up into added files. Tried to come up with some sort of hierarchy pattern for the main entity/controller/plugin classes.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Confirmed that after applying the patch, visiting the front page dies with a fatal error but running update.php fixes it.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Commited and pushed.

Status: Fixed » Closed (fixed)

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