8000 [DependencyInjection] Fix tests of #18144 by HeahDude · Pull Request #18422 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DependencyInjection] Fix tests of #18144 #18422

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

[DependencyInjection] Fix tests of #18144 #18422

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85d4ac4
fix tests of #18144
HeahDude Apr 3, 2016
fdbbb1a
missing test ?
HeahDude Apr 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 1 addition & 15 deletions src/Symfony/Component/DependencyInjection/Config/AutowireServiceResource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
namespace Symfony\Component\DependencyInjection\Config;

use Symfony\Component\Config\Resource\SelfCheckingResourceInterface;
use Symfony\Component\DependencyInjection\Compiler\AutowirePass;

class AutowireServiceResource implements SelfCheckingResourceInterface, \Serializable
{
Expand All @@ -33,20 +32,7 @@ public function isFresh($timestamp)
return false;
}

// has the file *not* been modified? Definitely fresh
if (@filemtime($this->filePath) <= $timestamp) {
return true;
}

try {
$reflectionClass = new \ReflectionClass($this->class);
} catch (\ReflectionException $e) {
// the class does not exist anymore!

return false;
}

return AutowirePass::createResourceForClass($reflectionClass);
return @filemtime($this->filePath) <= $timestamp;
Copy link
Member

Choose a reason for hiding this comment

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

@HeahDude thanks for fixing the test failure on #18144 - I didn't realize I had left an error!

But, this is wrong! This simplifies this resource down to be basically identical to the SelfCheckingResourceInterface.
The original last line was a mistake - I added a bug in my last commit - this was the mistake: 49271e8#diff-04e2904117929f6ffe784d5be3cb1103R49

So, this should be reverted, but and the last line corrected to be:

return AutowirePass::createResourceForClass($reflectionClass); == $this;

Could you make a PR for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@weaverryan Yes, I tried to figure out what to do, I was not sure about that "easy" way :)

Thanks for the lead, I'm on it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@weaverryan Is there anyway to add a test for that ?

}

public function __toString()
Expand Down
0