-
-
Notifications
You must be signed in to change notification settings - Fork 825
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/core#1615 - Import civicrm-setup #16618
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Note: This does not import the docs. Those should go to the dev guide.
(Standard links)
|
totten
added a commit
to totten/civicrm-wordpress
that referenced
this pull request
Feb 24, 2020
Overview -------- Fix a recent (intra-alpha) regression from civicrm/civicrm-core#16618 The search for `civicrm-setup` goes slightly awry because the migrated copy of civicrm-setup uses the core's classloader rather than its own classloader. Steps to Reproduce ------------------ * Create a site with WP and Civi(master). * Ensure Civi is uninstalled. (ex: `cv core:uninstall -f`) * In `wp-content/plugins/civicrm/civicrm`, touch the file `.use-civicrm-setup` * Go to the WP admin console. In the left navbar, choose "CiviCRM Installer". Before ------ The search for `civicrm-setup` fails to use the code in `./civicrm/setup` because `civicrm-setup-autoload.php` is not provided or necessary. It displays the old UI. After ----- It does find the copy of `./civicrm/setup` and displays the new UI. The absence of `civicrm-setup-autoload.php` is no decisive. Technical Details -----------------
totten
added a commit
to totten/civicrm-core
that referenced
this pull request
Feb 24, 2020
Overview -------- Fix a recent (intra-alpha) regression from civicrm#16618 The search for `civicrm-setup` goes slightly awry because the migrated copy of civicrm-setup uses the core's classloader rather than its own classloader. Steps to Reproduce ------------------ * Create a site with WP and Civi(master). * Ensure Civi is uninstalled. (ex: `cv core:uninstall -f`) * In `wp-content/plugins/civicrm/civicrm`, touch the file `.use-civicrm-setup`. (*This is supposed to activate the new UI.*) * Go to the WP admin console. In the left navbar, choose "CiviCRM Installer". Before ------ It fails to load the new UI -- the search finds `.use-civicrm-setup` but does not find `civicrm-setup-autoload.php`. It falls back to the old UI. After ----- It opens the new UI.
Testing on WP showed a small issue which can be resolved by #16621 |
magnolia61
pushed a commit
to magnolia61/civicrm-core
that referenced
this pull request
Mar 3, 2020
Overview -------- Fix a recent (intra-alpha) regression from civicrm#16618 The search for `civicrm-setup` goes slightly awry because the migrated copy of civicrm-setup uses the core's classloader rather than its own classloader. Steps to Reproduce ------------------ * Create a site with WP and Civi(master). * Ensure Civi is uninstalled. (ex: `cv core:uninstall -f`) * In `wp-content/plugins/civicrm/civicrm`, touch the file `.use-civicrm-setup`. (*This is supposed to activate the new UI.*) * Go to the WP admin console. In the left navbar, choose "CiviCRM Installer". Before ------ It fails to load the new UI -- the search finds `.use-civicrm-setup` but does not find `civicrm-setup-autoload.php`. It falls back to the old UI. After ----- It opens the new UI.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Overview
This migrates
civicrm-setup.git
, which is a refactored version of theinstall/
component. The library originated as an external package so that it could go through rapid iteration. It's been pretty stable/unchanging for the past year. For more complete rationale, see:https://lab.civicrm.org/dev/core/issues/1615
Before
civicrm-setup
has its own git repo (https://github.com/civicrm/civicrm-setup/) and is loaded viacomposer
After
civicrm-setup
lives withincivicrm-core.git:setup/
Technical Details
civicrm-wordpress and cv both use a search algorithm to locate
civicrm-setup
. The search algorithm checks both the old and new locations, so that's handy (i.e. this doesn't require coordinated updates in the other repos).Comments
I've done some light testing with
cv core:install
. After the PR test runs, I'll try some testing for other cases which use it - e.g. D8 test builds and CivI-WP (w/.use-civicrm-setup
).The
civicrm-setup.git
repo also has docs. These should be migrated tocivicrm-dev-docs.git
.