Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/drupal#4 - Add Civi\Setup::getPendingAction() helper #16628

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

totten
Copy link
Member

@totten totten commented Feb 26, 2020

Overview

This exposes a new piece of information for downstream consumers of the civicrm-setup API.

It doesn't change any behaviors or expectations - it merely tracks an extra piece of info.

Before

The Civi\Setup interface does not give any reporting about what's going on.

After

The Civi\Setup interface has a method, getPendingAction(), which reports the pending action.

Technical Details

(1) The basic gist of the change - in each of the installer actions (installFiles, installDatabase, uninstallDatabase, uninstallFiles), the patch sets the $pendingAction at the start and unsets it at the end. This requires wrapping the code in a try... finally... block (which makes the patch look a little longer than it should).

(2) The installer actions are strictly singular - it would not be sensible, for example, to have the installFiles and uninstallFiles actions running at the same time. Thus, the guards within each function.

(3) This facilitates dev/drupal#4 and civicrm/civicrm-drupal-8#37 - there were some use-cases in which the auto-install behaviors of civicrm_install() and FlushDrupal8.civi-setup.php would provoke each other (causing a sort of installer-loop). This change makes it possible to avoid the loop without changing the contracts.

Overview
--------

This exposes a new piece of information for downstream consumers of the `civicrm-setup` API.

It doesn't change any behaviors or expectations - it merely tracks an extra piece of info.

Before
------

The `Civi\Setup` interface does not give any reporting about what's going on.

After
-----

The `Civi\Setup` interface has a method, `getPendingAction()`, which reports the pending action.

Technical Details
-----------------

(1) The pending action is strictly singular - it would not be sensible, for example, to have the
`installFiles` and `uninstallFiles` actions running at the same time. Thus, the guards within
each function.

(2) The basic gist of the change - in each of the functions
(`{install,uninstall}{Files,Database}`), set the `pendingAction` at the start and unset
it at the end.  The change looks slightly bigger than it should - because it wraps the
existing code within the `try ...  finally ...` block.

(3) This facilitates dev/drupal#4 and civicrm/civicrm-drupal-8#37
- there were some use-cases in which the auto-install behaviors of `civicrm_install()`
and `FlushDrupal8.civi-setup.php` would provoke each other (causing a sort of
installer-loop).  This change makes it possible to avoid the loop without changing the
contracts.
@civibot
Copy link

civibot bot commented Feb 26, 2020

(Standard links)

@civibot civibot bot added the master label Feb 26, 2020
@totten
Copy link
Member Author

totten commented Feb 26, 2020

jenkins, test this please

@seamuslee001
Copy link
Contributor

This looks fine to me adding merge on pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants