-
Notifications
You must be signed in to change notification settings - Fork 29
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
New cv upgrade command #2
Conversation
@agh1 I've added a few small fixes. (Described in commit notes.) One blocker issue in
Suggestion: all variables passed through to |
When this is merged, it'd be nice to add an answer to this SE question which mentions the new command. |
1. Move hard-coded URLs to constants 2. Fix weird function name 3. Return exit code from UpgradeGetCommand
(1) It's confusing that several `Upgrade*Command`s show boot-related options when they don't actually boot. (2) It's peculiar/inefficient that the `upgrade:report` launches so many subprocesses (`Cv::run()`). However, it makes sense if you consider that the `upgrade:report` needs to run inside a *non-bootstrapped* environment.
On OSX, the default behavior of rand() only produces 31 bits of entropy. MD5 hashing is 128-bits, so we've left a lot of gaps in the 128-bit space. (Effectively: 93 vs 128 bits.) The PHP docs say that rand() on Windows gives even less entropy. This revision ensures a more consistent amount of entropy.
…ctively or (b) verbosely. Support dry-runs.
I haven't gone through all these carefully yet, but I'd like to be able to merge so that UpgradeDbCommand works.
@agh1 I've made a few updates on this PR
|
@seanmadsen Posted a comment on https://civicrm.stackexchange.com/a/22949/93 about the |
Mainly in prep for https://issues.civicrm.org/jira/browse/CRM-19417