Skip to content

Only enforce schema limits for supported apps - #10555

Merged
blizzz merged 3 commits into
masterfrom
bugfix/10518/only-check-added-items
Jan 3, 2019
Merged

Only enforce schema limits for supported apps#10555
blizzz merged 3 commits into
masterfrom
bugfix/10518/only-check-added-items

Conversation

@nickvergessen

@nickvergessennickvergessen commented Aug 6, 2018

Copy link
Copy Markdown
Member

@nickvergessennickvergessen mentioned this pull request Aug 6, 2018
Closed
rullzer
rullzer reviewed Aug 6, 2018
View reviewed changes
Comment threadlib/private/DB/MigrationService.php
$infoParser = new InfoParser();
$info = $infoParser->parse($appPath . '/appinfo/info.xml');
if (!isset($info['dependencies']['database'])) {
$this->checkOracle = true;

@rullzerrullzerAug 6, 2018

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.

so by default we check always check oracle?

@nickvergessennickvergessenAug 6, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yeah if the author did not define a db, you can install it on any. It a db is given the dependency check ensures it. So that mathes the logic here

@rullzerrullzer added this to the Nextcloud 15 milestone Aug 8, 2018
@rullzer

rullzer commented Aug 8, 2018

Copy link
Copy Markdown
Member

Lets do this properly for 15

@rullzerrullzer mentioned this pull request Aug 8, 2018
Closed
@MorrisJobke

MorrisJobke commented Oct 2, 2018

Copy link
Copy Markdown
Member

@nickvergessen Could you please rebase to fix the conflict?

@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 2, 2018
@nickvergessen

nickvergessen commented Oct 8, 2018•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Done, the "conflict" was f5c63d7

@nickvergessen
nickvergessenforce-pushed the bugfix/10518/only-check-added-items branch from bba82f1 to 9af5d4cCompareOctober 8, 2018 10:58
@nickvergessennickvergessen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 8, 2018
MorrisJobke
MorrisJobke approved these changes Oct 9, 2018
View reviewed changes

@MorrisJobkeMorrisJobke left a comment

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.

Tested and works with shipped apps 👍

@MorrisJobke

MorrisJobke commented Oct 9, 2018

Copy link
Copy Markdown
Member

Looks like some tests fail 🙈

@nickvergessen
nickvergessenforce-pushed the bugfix/10518/only-check-added-items branch from 9af5d4c to cc1253aCompareOctober 10, 2018 08:45
@MorrisJobke

MorrisJobke commented Nov 2, 2018

Copy link
Copy Markdown
Member

Looks like some tests fail 🙈

Resolved.

@MorrisJobke

MorrisJobke commented Nov 5, 2018

Copy link
Copy Markdown
Member

@rullzer @danielkesselberg @blizzz Mind to review this one?

@MorrisJobke

MorrisJobke commented Nov 7, 2018

Copy link
Copy Markdown
Member

@nickvergessen@rullzer Should we do this now or wait for 16?

@MorrisJobkeMorrisJobke mentioned this pull request Nov 7, 2018
Merged
29 tasks
@nickvergessen

nickvergessen commented Nov 7, 2018

Copy link
Copy Markdown
MemberAuthor

I'm always in favor of it. but well I manually cross check that oracle still works via the notifications tests…

@MorrisJobkeMorrisJobke mentioned this pull request Nov 8, 2018
Merged
24 tasks
@MorrisJobke

MorrisJobke commented Nov 13, 2018

Copy link
Copy Markdown
Member

Let's move it to 16 and merge it early this time.

@nickvergessen

nickvergessen commented Nov 14, 2018

Copy link
Copy Markdown
MemberAuthor

#12446 🙈

@nickvergessen
Only check changed items
f265657
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Only check the Oracle schema conditions if the app supports it
c09fa1e
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Update the tests to the comparison logic
85a0e10
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessenforce-pushed the bugfix/10518/only-check-added-items branch from cc1253a to 85a0e10CompareDecember 17, 2018 12:52
@nickvergessen

nickvergessen commented Dec 17, 2018

Copy link
Copy Markdown
MemberAuthor

Rebased to re-execuite the tests, please merge soonish

@blizzz
blizzz merged commit 6895230 into masterJan 3, 2019
@blizzz
blizzz deleted the bugfix/10518/only-check-added-items branch January 3, 2019 13:18
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzerrullzerrullzer left review comments
@MorrisJobkeMorrisJobkeMorrisJobke approved these changes
@blizzzblizzzblizzz approved these changes
@icewind1991icewind1991Awaiting requested review from icewind1991
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

No one assigned

Labels

3. to reviewWaiting for reviewsbug

Projects

None yet

Milestone

Nextcloud 16

Development

Successfully merging this pull request may close these issues.

4 participants

@nickvergessen@rullzer@MorrisJobke@blizzz