Problem/Motivation

standard_install() manually updates various bits of configuration instead of shipping with proper configuration exports in config/install. This promotes a bad practice for other installation profile authors.

Proposed resolution

Remove all configuration updates in favor of exported configuration files.

CommentFileSizeAuthor
#9 2990776-9.patch6.97 KBtstoeckler
#9 2990776-6-9-interdiff.txt353 byteststoeckler
#6 2990776-6.patch6.85 KBtstoeckler
#6 2990776-4-6-interdiff.txt1.49 KBtstoeckler
#4 2990776-4.patch5.35 KBtstoeckler
#4 2990776-2-4-interdiff.txt1.14 KBtstoeckler
#2 2990776-2.patch5.24 KBtstoeckler

Comments

tstoeckler created an issue. See original summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review
StatusFileSize
new 2990776-2.patch5.24 KB

Here we go.

I could have sworn we already an issue for this somewhere, but couldn't find anything. I put some related prior art in the "Related issues" section, but I didn't find anything about this part in particular.

Just handling Standard here, to see how the issue goes. Umami suffers from the same problem, will open a separate issue for that.

Status: Needs review » Needs work

The last submitted patch, 2: 2990776-2.patch, failed testing. View results

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review
StatusFileSize
new 2990776-2-4-interdiff.txt1.14 KB
new 2990776-4.patch5.35 KB

Aborted the test for #2 because I realized I failed to account for node_install() and FilterFormat::postSave(). (Also fixes an incorrect ordering.)

Status: Needs review » Needs work

The last submitted patch, 4: 2990776-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review
StatusFileSize
new 2990776-4-6-interdiff.txt1.49 KB
new 2990776-6.patch6.85 KB

Well that's easy enough to fix...

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Needs review » Needs work

This is really cool and cleans up some code.

I only found one nitpick:

  • The RoleInterface can now be removed from the use statements in standard.install
chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Issue tags: +Needs followup

We should also open a follow-up to do these changes in umami as well.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review
Issue tags: -Needs followup
StatusFileSize
new 2990776-6-9-interdiff.txt353 bytes
new 2990776-9.patch6.97 KB

Here we go. Will open the follow-up now, and reference this one, so it should appear in the sidebar in a minute ;-)

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Needs review » Reviewed & tested by the community

Thank you. 👍

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 44920d2 and pushed to 8.7.x. Thanks!

  • catch committed 44920d2 on 8.7.x 
    Issue #2990776 by tstoeckler, chr.fritsch: Remove config-editing parts...
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Awesome, thank you! Just posted a patch in #2357215: Clean up hook_install() in Standard Install Profile which is now pretty easy...

Status: Fixed » Closed (fixed)

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