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

(release/1) Allow set-version.php to generate upgraders for major/minor increments #11731

Merged
merged 3 commits into from
Feb 28, 2018

Conversation

totten
Copy link
Member

@totten totten commented Feb 27, 2018

Overview

The utility, tools/bin/scripts/set-version.php, is used to generate
boilerplate files for incremental updates. This change improves
compatibility with the version realignment.

Before

The set-version.php script supports incrementing the third digit only.

Specifically, it:

  • Sets the current version number in xml/version.xml (etal). (This is required for all version bumps.)
  • Generates an incremental SQL upgrade file. (This is required for all version bumps.)

After

The set-version.php script supports incrementing the first, second, and/or third digit.

Specifically, it generates the same files as before, and:

  • Generates an incremental PHP upgrade file. (This is only required when increasing the first or second digit.)

Comments

To test this, I locally produced a series of upgrade steps:

./tools/bin/scripts/set-version.php 5.0.alpha1 --no-commit
./tools/bin/scripts/set-version.php 5.0.alpha2 --no-commit
./tools/bin/scripts/set-version.php 5.0.0 --no-commit
./tools/bin/scripts/set-version.php 5.0.1 --no-commit
./tools/bin/scripts/set-version.php 5.1.alpha1 --no-commit
./tools/bin/scripts/set-version.php 5.1.beta1 --no-commit

Then, I ran cv upgrade:db -vvv (as
both dry-run and normal-run) and verified that the expected upgrade steps
were used.

Currently, this file is produced by GenCode, so it's not critical to update it via `set-version.php`. However, it's
a bit easier to test the process if this file is kept up-to-date.

In the near future, this file may be reworked to read from the canonical
source.  However, the search/replace should still be safe then.
@eileenmcnaughton
Copy link
Contributor

I'm merging this based on @totten's testing (described above) & a skim of the code. I believe that any impact this has would flush itself out in the rc process & on dev installs & the change seems fairly trivial.

I note this does not commit us to the version re-alignment which I support as long as all messaging at the time is around this being no change from the normal process other than a minor renaming. There is a prospect for confusion & I think that if we can't keep that message clear & consistent the case for the version re-alignment cannot be made.

@eileenmcnaughton eileenmcnaughton merged commit 7585c51 into civicrm:master Feb 28, 2018
@totten totten deleted the master-setver branch February 28, 2018 03:37
@mlutfy mlutfy added this to the 4.7.32 milestone Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants