Skip to content

Return null from migration to match with typehint - #17740

Merged
rullzer merged 1 commit into
masterfrom
fix/17479/always-return-schema
Oct 29, 2019
Merged

Return null from migration to match with typehint#17740
rullzer merged 1 commit into
masterfrom
fix/17479/always-return-schema

Conversation

@kesselb

@kesselbkesselb commented Oct 29, 2019

Copy link
Copy Markdown
Contributor

@kesselb

Copy link
Copy Markdown
ContributorAuthor

/backport to stable17

@nickvergessen

Copy link
Copy Markdown
Member

Why? The schema should only be returned when it actually changed something. Which is only the case inside the if?

Otherwise this locks the db starts heavy diffing of the schema just to unlock again?

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb
kesselbforce-pushed the fix/17479/always-return-schema branch from 5dcae6e to fd83661CompareOctober 29, 2019 18:25
@kesselb

Copy link
Copy Markdown
ContributorAuthor

The schema should only be returned when it actually changed something. Which is only the case inside the if?

OK. It returns null outside the if now.

@kesselbkesselb changed the title Always return $schemaReturn null from migration to match with typehintOct 29, 2019
$table->addIndex(['object_id'], 'calendar_reminder_objid');
$table->addIndex(['uid', 'recurrence_id'], 'calendar_reminder_uidrec');

return $schema;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep this

return $schema;
}

return $schema;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return null here

@rullzer
rullzer merged commit 1a9f551 into masterOct 29, 2019
@rullzer
rullzer deleted the fix/17479/always-return-schema branch October 29, 2019 19:38
@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable17 in #17742

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

Labels

3. to reviewWaiting for reviewsbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade from 16.0.5 to 17.0.0 fails (apps/dav migration crashes)

3 participants

@kesselb@nickvergessen@rullzer