In #1615520: Example module for the new Drupal 8 configuration system: new module needed we want to create a new example module for the config system, but to truly demonstrate the upgrade path we will need to have a similar module in D7 (suggested by rfay in comment #10 of the above-mentioned issue).

Files: 
CommentFileSizeAuthor
#8 1630762-8-config-module.patch24.86 KBalberto56
PASSED: [[SimpleTest]]: [MySQL] 2,539 pass(es).
[ View ]
#5 1630762-5-D7-config-module.patch20.1 KBalberto56
PASSED: [[SimpleTest]]: [MySQL] 2,525 pass(es).
[ View ]
#3 1630762-3-D7-config-module.patch20.1 KBalberto56
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/examples/configuration_example/configuration_example.test.
[ View ]
#1 1630762-1-D7-config-module.patch47.99 KBalberto56
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1630762-1-D7-config-module.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new47.99 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1630762-1-D7-config-module.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here is a patch

Status:Needs review» Needs work

The last submitted patch, 1630762-1-D7-config-module.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new20.1 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/examples/configuration_example/configuration_example.test.
[ View ]

Here is another one.

Status:Needs review» Needs work

The last submitted patch, 1630762-3-D7-config-module.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new20.1 KB
PASSED: [[SimpleTest]]: [MySQL] 2,525 pass(es).
[ View ]

oops, syntax error

Status:Needs review» Needs work

@alberto56, thanks for this! I think you're well on the way.

I wasn't able to understand your intent by just reading the code; the README.txt helped, but the code didn't really explain to me what you were doing.

Could you
1. Simplify the database-based configuration so you can just focus on the fact that it's configuration.
2. Improve the comments to explain that you're using the database table for configuration (and how it changes the behavior of the module)

Please begin the .module file with an explanation of what you're trying to do (the info you put into the README).

Please move the non-hook functions below the hook functions.

When you do the database-based configuration, please explain what it is that is happening.

Again, thanks very much for this, and sorry for the slow review!

@rfay thanks for the feedback. I have started work on this, and will provide an updated patch when I return from my vacation mid-July.

Cheers,

Albert.

Assigned:Unassigned» alberto56
Status:Needs work» Needs review
StatusFileSize
new24.86 KB
PASSED: [[SimpleTest]]: [MySQL] 2,539 pass(es).
[ View ]

Hi,

Here is a new patch which takes rfay's considerations into account. Here are a few notes:

(1) "Simplify the database-based configuration": I have removed the kitten "size", leaving only the name, machine name, and color. These are all important in my opinion: the machine name is automatically derived from the name -- this is standard throughout the Drupal interface I think; and the color exists so that we can associated two (or more) pieces of data, which is what complex configuration usually does. If anyone has any concrete ideas as to how I can simplify this more, please let me know!

(2) I have added comments everywhere to explain further what I am trying to accomplish. If any passage or code appears unclear, please specify which. Specifically, @rfay if you still would like clarification on database-based configuration, please refer to precise passages.

(3) Function order: Hook functions, then non-hook functions.

(4) Added a new test to confirm that deleting one item does not delete others as well.

(5) All tests pass and have been tested.

(6) These improvements have also been included in a new version (comment #15) of the patch in #1615520: Example module for the new Drupal 8 configuration system: new module needed

(7) Repaired an issue, and added a test, where deleting a set xyz from the complex configuration caused a erroneous message "xyz does not exist" to appear on-screen.

Cheers,

Albert.

Status:Needs review» Active

In light of #1699440: A dedicated to module to demonstrate hook_update_N() and automatically testing upgrades and updates, I suggest setting this issue to "not fixed". This patch's reason for being was only to demonstrate the upgrade path to #1615520: Example module for the new Drupal 8 configuration system: new module needed, but it seems clunky to me now -- the upgrade path should be demonstrated in a separate example module.

Thoughts?