-
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
Use a separate class for reading info.xml #9803
Conversation
@@ -0,0 +1,73 @@ | |||
<?php | |||
/** | |||
* Copyright (c) 2013 Bart Visscher <bartv@thisnet.nl> |
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.
back to the future! 😉
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.
Well it is really written last year, trying to get this code merged. But in smaller chunks now
🚀 Test Passed. 🚀 |
Any comments? |
👍 |
@DeepDiver1975 @PVince81 one more 👍 ? |
@bartv2 sorry for the late review |
*/ | ||
public function getInfo($filename) { | ||
if (!file_exists($filename)) { | ||
return null; |
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.
I'm not so sure about returning null in case of failure, especially if there can be multiple failure cases
fixed all review comments |
if (!$content) { | ||
throw new Exception('info.xml file is empty or unreadable: '.$filename); | ||
} | ||
$xml = new \SimpleXMLElement($content); |
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.
owncloud-archive/3rdparty#117 @LukasReschke
This should be merged together :)
🚀 Test Passed. 🚀 |
@bartv2 please rebase |
it basically reparses the file and not just returns a cached version
6696fdb
to
56e576c
Compare
rebased |
The inspection completed: No issues found |
💣 Test Failed. 💣 |
💣 Test FAILed. 💣 |
💣 Test FAILed. 💣 |
💣 Test FAILed. 💣 Build result: FAILURE[...truncated 123 lines...] > git fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse 56e576c^{commit} # timeout=10Checking out Revision 56e576c (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 56e576c > git rev-list 28dcb5277015c13914cd6a55c57fde37c583a8fb # timeout=10First time build. Skipping changelog. > git remote # timeout=10 > git submodule init # timeout=10 > git submodule sync # timeout=10 > git config --get remote.origin.url # timeout=10 > git submodule update --init --recursiveCleaning workspace > git rev-parse --verify HEAD # timeout=10Resetting working tree > git reset --hard # timeout=10 > git clean -fdx # timeout=10 > git submodule foreach --recursive git reset --hard # timeout=10 > git submodule foreach --recursive git clean -fdx # timeout=10Triggering pull-request-analyser-ng-simple » vm-slave-02Configuration pull-request-analyser-ng-simple » vm-slave-02 is still in the queue: Waiting for next available executor on vm-slave-02pull-request-analyser-ng-simple » vm-slave-02 completed with result FAILUREStarted calculate disk usage of buildDisk usage plugin fails during calculation disk usage of this build.💣 Test FAILed. 💣 |
Same as #12393 ? |
superseded by #12393 -> close |
@DeepDiver1975 @butonic @Raydiation @PVince81