Skip to content

Fix multiple handler callbacks in YamlDriver #515

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 2 commits into from
Aug 12, 2016

Conversation

mpajunen
Copy link
Contributor

Using handler callbacks for multiple formats caused errors in YamlDriver because the direction key was parsed multiple times. Now direction key is only parsed once (per direction).

A basic test case for handler callbacks is included for all configuration formats.

@mpajunen
Copy link
Contributor Author

ping @schmittjoh

Would be nice to see this merged, the fix is simple and obvious. If the new tests need some more work, I'll be happy to improve them.

The test failures seem unrelated and are probably fixed by #511 in any case.

@mpajunen mpajunen force-pushed the yaml-driver-handler-callbacks branch from 0388346 to b044dd0 Compare May 13, 2016 13:17
@mpajunen
Copy link
Contributor Author

Rebased, tests are now green as well.

@@ -204,8 +204,8 @@ protected function loadMetadataFromFile(\ReflectionClass $class, $file)

if (isset($config['handler_callbacks'])) {
foreach ($config['handler_callbacks'] as $direction => $formats) {
$direction = GraphNavigator::parseDirection($direction);
Copy link
Contributor

Choose a reason for hiding this comment

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

as this store a different thing, it would be even better to use a different variable name instead of overwriting the variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Loop variable name changed to $directionName.

@mpajunen mpajunen force-pushed the yaml-driver-handler-callbacks branch from b044dd0 to dcaf30a Compare May 13, 2016 17:25
@goetas goetas added the To Merge label Aug 4, 2016
@goetas goetas added this to the v1.3 milestone Aug 4, 2016
@goetas
Copy link
Collaborator

goetas commented Aug 4, 2016

will be part of v1.3, tnx

@goetas goetas merged commit 1591c5c into schmittjoh:master Aug 12, 2016
@mpajunen mpajunen deleted the yaml-driver-handler-callbacks branch August 13, 2016 08:01
@mpajunen
Copy link
Contributor Author

Nice to see this merged finally, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants