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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the way to check for initial migration #3487

Merged
merged 3 commits into from Dec 12, 2017

Conversation

Projects
None yet
3 participants
@j0k3r
Copy link
Member

j0k3r commented Dec 12, 2017

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Documentation no
Translation no
CHANGELOG.md 馃槙
Fixed tickets #3480
License MIT

Instead of checking if some migrations were run we just check if the main table exist.

  • if it exists, we can skip the initial migration
  • if not, run the initial migration

Poke @aaa2000

@j0k3r j0k3r added this to the 2.3.1 milestone Dec 12, 2017

@j0k3r j0k3r requested review from nicosomb , Kdecherf and tcitworld Dec 12, 2017

@aaa2000

This comment has been minimized.

Copy link
Contributor

aaa2000 commented Dec 12, 2017

I don't understand why the migration_versions table is empty. Do you have an idea ?

The initial migration will not failed but the next migration will fail, no ?

@aaa2000

This comment has been minimized.

Copy link
Contributor

aaa2000 commented Dec 12, 2017

Maybe, I understand. The next migration use $this->skipIf(.., so if all migrations use the same behaviour, the migration_versions table will be empty.

$this->skipIf($entryTable->hasColumn('uid') || $entryTable->hasColumn('uuid'), 'It seems that you already played this migration.');

/**
* @param Schema $schema
*/
public function up(Schema $schema)
{
if ($this->version->getConfiguration()->getNumberOfExecutedMigrations() > 0) {
$this->version->markMigrated();
try {

This comment has been minimized.

@aaa2000

aaa2000 Dec 12, 2017

Contributor

You can mayble use $schema->hasTable($this->getTable('entry')); instead of $schema->getTable($this->getTable('entry'));

@j0k3r

This comment has been minimized.

Copy link
Member

j0k3r commented Dec 12, 2017

鈽濓笍 that doc linking is a mistake.

@aaa2000
Copy link
Contributor

aaa2000 left a comment

Sorry for htr bug, I can't test now but it looks good.

Need fix phpcs app/DoctrineMigrations/Version20160401000000.php (no_unused_imports)

CS
@j0k3r

This comment has been minimized.

Copy link
Member

j0k3r commented Dec 12, 2017

@aaa2000 you're right, sh*t

@nicosomb nicosomb merged commit 1f19825 into master Dec 12, 2017

5 checks passed

Scrutinizer No new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
zappr/pr/specification PR has passed specification checks
zappr/pr/tasks PR has no open tasks.

@nicosomb nicosomb deleted the initial-migration branch Dec 12, 2017

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