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

[12] Properly encapsulate require_once for app.php #8632

Merged
merged 2 commits into from Mar 5, 2018

Conversation

juliushaertl
Copy link
Member

If an app has an appinfo/app.php file, that includes a variable called $app, this would lead to a type error when calling getAppInfo otherwise.

Backport of #8372

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@codecov
Copy link

codecov bot commented Mar 2, 2018

Codecov Report

Merging #8632 into stable12 will increase coverage by <.01%.
The diff coverage is 33.33%.

@@              Coverage Diff               @@
##             stable12    #8632      +/-   ##
==============================================
+ Coverage       53.92%   53.92%   +<.01%     
  Complexity      22764    22764              
==============================================
  Files            1387     1387              
  Lines           87251    87251              
  Branches         1331     1331              
==============================================
+ Hits            47050    47051       +1     
+ Misses          40201    40200       -1
Impacted Files Coverage Δ Complexity Δ
lib/private/legacy/app.php 52.8% <33.33%> (ø) 219 <0> (ø) ⬇️
lib/private/Server.php 84.68% <0%> (-0.13%) 121% <0%> (ø)
core/js/js.js 61.94% <0%> (+0.11%) 0% <0%> (ø) ⬇️
lib/private/Security/CertificateManager.php 92.07% <0%> (+0.99%) 39% <0%> (ø) ⬇️

@@ -133,6 +133,7 @@ public static function loadApps($types = null) {
* load a single app
*
* @param string $app
* @throws Exception
Copy link
Member

Choose a reason for hiding this comment

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

Is this still true?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. This is another exception outside of this try catch.

@nickvergessen nickvergessen merged commit af8d300 into stable12 Mar 5, 2018
@MorrisJobke MorrisJobke deleted the stable12-8372 branch March 5, 2018 13:22
@MorrisJobke MorrisJobke mentioned this pull request Mar 9, 2018
2 tasks
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.

None yet

4 participants