Merge farm_settings module into farm_setup #1024
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This merges the
farm_settingsmodule intofarm_setup, uninstallsfarm_settingsvia an update hook, and marks it asobsolete.A companion branch against farmOS
4.xdoes the same thing, but also removes thefarm_settingsmodule and the update hook (adding its name tohook_removed_post_updates()to ensure hosts upgrade to the latest 3.x release before 4.x - see #962): 4.x...mstenta:farmOS:4.x-merge-settings-into-setupThe motivation for doing this is twofold:
farm_settings, but we are considering adding a "Setup Wizard", which would be infarm_setup, so it feels logical for them to be in the same module. We can't makefarm_setupdepend onfarm_settingsbecause that would create a circular dependency.This was briefly considered when the
farm_setupmodule was first added in #706. See: #706 (comment)Right now they are both pretty simple.
farm_setupbasically just provides the "Setup" main menu item, andfarm_settingsdepends onfarm_setup(for that menu item) and adds two forms: the settings form+menu item and the module installation form. So as it is, it's possible to havefarm_setupinstalled withoutfarm_settings, but it isn't possible to havefarm_settingswithoutfarm_setup. They have separate permissions (access farm setupvsadminister farm settings) - so it's already possible to set up users with the ability to access Setup without accessing Setup > Settings.With all of that in mind, I don't see a strong reason to keep them separate, and merging them simplifies next steps for the setup wizard work.
PS: There is a second commit which simply moves the Gin-specific logic of adding the
form-checkboxesclass to the modules form over to thefarm_ui_thememodule, which is responsible for encapsulating all Gin-specific stuff.