[TwigBundle] Reconfigure twig paths when they are updated #14773
Conversation
|
@chbruyand just FYI, the email you used to commit is not associated to your github account |
|
Adding a new feature in the Config component in 2.3 would be really weird IMO (and a pain in the ass to write requirements on the component when needing this feature) |
| */ | ||
| public function __construct($resource) | ||
| { | ||
| $this->resource = realpath($resource); |
There was a problem hiding this comment.
the usage of realpath means it is not usable inside phars
There was a problem hiding this comment.
OK.
realpath() is also used in Config\Resource\FileResource, do you suggest removing it also there ?
|
IMO, you don't need both FileExistenceResource and FileAbsenceResource but a single FileExistenceResource. In the constructor, you should register the path (passed as argument) and whether the file exists. In |
| * | ||
| * @author Charles-Henri Bruyand <charleshenri.bruyand@gmail.com> | ||
| */ | ||
| class FileAbsenceResource implements ResourceInterface, \Serializable |
There was a problem hiding this comment.
Is is necessary to implement \Serializable?
There was a problem hiding this comment.
@mpdude it makes the meta file much smaller when serializing resources
|
Agree to @stof's comment above. You might even just pass in the path and the resource itself checks whether or not it exists (and will repeat the same check in the future when freshness is checked). |
|
IMO, adding new resource types should be done in the 2.8 only, to avoid breaking semver. @fabpot it looks like there is an issue with fabbot.io. It says "Not Found" as status. |
|
I removed usage of About your concerns for the Config component, what do you suggest ? Adding |
| private $exists; | ||
|
|
||
| /** | ||
| * Constructor. |
There was a problem hiding this comment.
I'd just remove this comment. It doesn't bring any value.
There was a problem hiding this comment.
Should I do the same in other Resources ? (same question for missing docblock)
There was a problem hiding this comment.
No, it's not part of your PR. I just don't want to introduce new comments like this.
|
Made a mistake, sorry for the noise. #14778 |
Directory's content and modification time are not considered any more for freshness check. Only it's creation or deletion are now checked.
This addresses performance issues (#14771, #14768), and also improves the cases were container is reconfigured as directory creation needed clearing the cache manually (#14262).