Skip to content

Do not load app.php if Application implements IBootstrap - #21812

Merged
MorrisJobke merged 1 commit into
masterfrom
feature/noid/app-bootstrap-without-app.php
Jul 14, 2020
Merged

Do not load app.php if Application implements IBootstrap#21812
MorrisJobke merged 1 commit into
masterfrom
feature/noid/app-bootstrap-without-app.php

Conversation

@MorrisJobke

@MorrisJobkeMorrisJobke commented Jul 13, 2020

Copy link
Copy Markdown
Member

The problem with this is that it changes the load order also for apps without the IBootstrap. Another approach would be a big warning if the app is then one with IBootstrap but still has an app.php.

#20573 and #20865

@MorrisJobkeMorrisJobke added this to the Nextcloud 20 milestone Jul 13, 2020
@MorrisJobke
MorrisJobkeforce-pushed the feature/noid/app-bootstrap-without-app.php branch from cbf0925 to 7d3c308CompareJuly 14, 2020 03:04
@ChristophWurst

ChristophWurst commented Jul 14, 2020

Copy link
Copy Markdown
Member

The problem with this is that it changes the load order also for apps without the IBootstrap

Why would it do that?

@MorrisJobke

MorrisJobke commented Jul 14, 2020

Copy link
Copy Markdown
MemberAuthor

Why would it do that?

Because I moved the IBootstrap call to bootApp above the include of the app.php. Thus the Application is loaded before the app.php. In the past it was the other way around. Sometimes this matter (if the autoloader is configured in the app.php vor example)

ChristophWurst
ChristophWurst reviewed Jul 14, 2020
View reviewed changes
Comment threadlib/private/legacy/OC_App.php Outdated
Comment threadlib/private/legacy/OC_App.php
@MorrisJobke
MorrisJobkeforce-pushed the feature/noid/app-bootstrap-without-app.php branch from 7d3c308 to d6339e7CompareJuly 14, 2020 12:23
@MorrisJobke

MorrisJobke commented Jul 14, 2020

Copy link
Copy Markdown
MemberAuthor

Ready for review :)

ChristophWurst
ChristophWurst approved these changes Jul 14, 2020
View reviewed changes

@ChristophWurstChristophWurst 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.

Looks great but I'd appreciate a more sane naming of this variable 🤡

Comment threadlib/private/legacy/OC_App.php Outdated
@MorrisJobke
Do not load app.php if Application implements IBootstrap
d9cc2f0
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobkeMorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 14, 2020
@MorrisJobke
MorrisJobkeforce-pushed the feature/noid/app-bootstrap-without-app.php branch from d6339e7 to d9cc2f0CompareJuly 14, 2020 13:56
@MorrisJobke
MorrisJobke merged commit fba64a9 into masterJul 14, 2020
@MorrisJobke
MorrisJobke deleted the feature/noid/app-bootstrap-without-app.php branch July 14, 2020 15:06
@ChristophWurstChristophWurst mentioned this pull request Jul 14, 2020
Closed
@GrotaxGrotax mentioned this pull request Sep 25, 2020
Closed
@icewind1991icewind1991 mentioned this pull request Oct 1, 2020
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurstChristophWurstChristophWurst approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@rullzerrullzerAwaiting requested review from rullzer
@kesselbkesselbAwaiting requested review from kesselb
@nickvergessennickvergessenAwaiting requested review from nickvergessen
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishenhancement

Projects

None yet

Milestone

Nextcloud 20

Development

Successfully merging this pull request may close these issues.

3 participants

@MorrisJobke@ChristophWurst@skjnldsv