Document why the backdrop_install_schema('system') in system_install() is necessary

[avpaderno]

system_install() calls backdrop_install_schema('system'), which is usually done from Backdrop core before installing a module.

Since this is an exception, a comment in the code should remember us that.

[avpaderno]

OK, I really missed something. 😅 I guess that a comment like This must be explicitly done, or the tables won't be created. should suffice.

[ghost]

I'm not sure extra comments are needed... It already says this creates tables.

[avpaderno]

It does not say why hook_schema() is explicitly called in hook_install(), when it is not necessary for other modules. That is what the documentation for hook_schema() says.

The tables declared by this hook will be automatically created when the module is first enabled, and removed when the module is uninstalled. This happens before hook_install() is invoked, and after hook_uninstall() is invoked, respectively.

IMO, a comment saying that is an exception to the "general rule" should be added, at least to make that clear to developers who watch at Backdrop core code to write their own code. If that is not considered necessary, I am fine with that.

[ghost]

Fair enough.

[avpaderno]

The sentence I added (This must be explicitly done or the tables won't be created.) is probably too terse. The longer version could be:

Differently from other modules, The System module needs to explicitly call backdrop_install_schema('system') or its tables won't be created.

I open to suggestions. (English is not my first language nor my second one.)

[argiepiano]

Differently from other modules, The System module needs to explicitly call backdrop_install_schema('system') or its tables won't be created.

I'm wondering why this explanation is needed? It is an interesting fact, but, as a developer, it probably won't change the way I do things. Knowing that hook_schema will be called before my module's install hook, on the other hand, is important and can change the way I create my code.

[laryn]

I think that it makes sense to document in a comment when there is a departure from the normal way of doing things and where the reason might not be immediately obvious.

[avpaderno]

The failing tests are not failing because of changes done in this PR. (I think I have seen those tests failing on other PRs too.)

[izmeez]

Differently from other modules, The System module needs to explicitly call backdrop_install_schema('system') or its tables won't be created.

How about:

As an exception from other modules, the System module needs to explicitly call backdrop_install_schema('system') or its tables won't be created.