-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
No max version check for apps when using release channel git or daily #33360
No max version check for apps when using release channel git or daily #33360
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see review. also CI unhappy.
@@ -312,7 +312,7 @@ private function analyzeOC(array $dependencies, array $appInfo) { | |||
$missing[] = (string)$this->l->t('ownCloud %s or higher is required.', $minVersion); | |||
} | |||
} | |||
if ($maxVersion !== null) { | |||
if ($maxVersion !== null && !\in_array(\OCP\Util::getChannel(), ['git', 'daily'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we usually add , true
at the end of in_array
for strict comparison.
wonder if our style checkers can verify this
lib/private/legacy/app.php
Outdated
@@ -936,6 +936,7 @@ public static function isAppCompatible($ocVersion, $appInfo) { | |||
} | |||
|
|||
if (!empty($requireMax) | |||
&& !\in_array(\OCP\Util::getChannel(), ['git', 'daily']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
b54f892
to
176a3a4
Compare
Codecov Report
@@ Coverage Diff @@
## master #33360 +/- ##
=============================================
+ Coverage 47.88% 64.06% +16.18%
- Complexity 0 18277 +18277
=============================================
Files 109 1190 +1081
Lines 10448 69044 +58596
Branches 1271 1271
=============================================
+ Hits 5003 44234 +39231
- Misses 5075 24440 +19365
Partials 370 370
Continue to review full report at Codecov.
|
23da368
to
7e8e859
Compare
@ownclouders rebase |
Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently |
7e8e859
to
7a2dd6c
Compare
Automated rebase with GitMate.io was successful! 🎉 |
@ownclouders rebase |
Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently |
7a2dd6c
to
49cf33f
Compare
Automated rebase with GitMate.io was successful! 🎉 |
65a741f
to
6b3eb0f
Compare
lib/private/legacy/app.php
Outdated
@@ -239,7 +245,7 @@ public static function isType($app, $types) { | |||
} | |||
$appTypes = self::getAppTypes($app); | |||
foreach ($types as $type) { | |||
if (\array_search($type, $appTypes) !== false) { | |||
if (\array_search($type, $appTypes, true)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for some strange reason, received shares don't appear in this PR.
Reverting this line to its previous state makes it work.
Maybe this disturbs the loading of apps by types and files_sharing maybe doesn't load at the right moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rechecked the code, looks good now 👍
@PVince81 I just added 3 more type safety fixes - will squash after CI being green ... THX |
fed93b2
to
a570d37
Compare
seems we missed some bits #33819 |
Description
When installing owncloud from git or the daily release tar ball the max version requirement is no longer checked when installing/enabling apps
This will help with the development and release of apps in general
@patrickjahns as discussed long time back - we did go for the release channel and no env variable.
Related Issue
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: