Skip to content

[FrameworkBundle] Add missing resource tracking for validation mapping #21927

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

Merged
merged 1 commit into from
Mar 9, 2017

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Mar 8, 2017

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #19086 (comment)
License MIT
Doc PR n/a

https://github.com/symfony/symfony/pull/21927/files?w=1

$files['yaml' === $extension ? 'yml' : $extension][] = $path;
} else {
throw new \RuntimeException(sprintf('Unsupported mapping type in "%s", supported types are XML & Yaml.', $path));
if ($container->fileExists($path)) {
Copy link
Member

Choose a reason for hiding this comment

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

content needs to be tracked for directories (but only for new XML and YAML files). But it does not need to be tracked for files (changing the content of the validation file does not need to invalidate the container). So this shoould be improved.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated. Could you please have a look?

if ($container->fileExists($path)) {
if (is_dir($path)) {
$this->getValidatorMappingFilesFromDir($path, $files);
} elseif (is_file($path)) {
Copy link
Member

Choose a reason for hiding this comment

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

else is enough here

@chalasr chalasr force-pushed the validation-resource-track branch 3 times, most recently from eccbc76 to 2435069 Compare March 8, 2017 21:31
@chalasr chalasr force-pushed the validation-resource-track branch from 2435069 to 0a19cbe Compare March 9, 2017 10:23
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

👍

@nicolas-grekas
Copy link
Member

Thank you @chalasr.

@nicolas-grekas nicolas-grekas merged commit 0a19cbe into symfony:master Mar 9, 2017
nicolas-grekas added a commit that referenced this pull request Mar 9, 2017
…tion mapping (chalasr)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[FrameworkBundle] Add missing resource tracking for validation mapping

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19086 (comment)
| License       | MIT
| Doc PR        | n/a

https://github.com/symfony/symfony/pull/21927/files?w=1

Commits
-------

0a19cbe Add missing resource tracking for validation mapping
@chalasr chalasr deleted the validation-resource-track branch March 9, 2017 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants