Skip to content

Conversation

@roippi
Copy link

@roippi roippi commented Nov 17, 2017

For #64. cc #169

Porting this was really just mechanical translation - kudos on making it so easy!

WIP because I've only done 1/3 of the tests or so. The rest of the code should be ready for review though, in case we want to get a head start on that.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.0%) to 91.667% when pulling 4ea6cbe on roippi:findbreakingchanges into eaadae4 on webonyx:master.

@vladar
Copy link
Member

vladar commented Nov 18, 2017

Awesome! I'll review a bit later and will merge as soon as you finish with tests. Thanks!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 92.263% when pulling 3c0ed78 on roippi:findbreakingchanges into eaadae4 on webonyx:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 92.794% when pulling 533b8b8 on roippi:findbreakingchanges into eaadae4 on webonyx:master.

@roippi roippi changed the title [WIP] port findBreakingChanges port findBreakingChanges Nov 21, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 92.847% when pulling b18dfd6 on roippi:findbreakingchanges into eaadae4 on webonyx:master.

@vladar
Copy link
Member

vladar commented Nov 28, 2017

So is it ready for review/merge?

@roippi
Copy link
Author

roippi commented Nov 28, 2017 via email

foreach ($oldTypeMap as $typeName => $typeDefinition) {
if (!isset($newTypeMap[$typeName])) {
$breakingChanges[] =
['type' => self::BREAKING_CHANGE_TYPE_REMOVED, 'description' => "${typeName} was removed."];
Copy link
Member

Choose a reason for hiding this comment

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

Had no idea that ${typeName} is a valid form of variable interpolation in PHP. Cool %)

@vladar vladar merged commit 9c563d5 into webonyx:master Nov 28, 2017
@vladar
Copy link
Member

vladar commented Nov 28, 2017

Thanks a lot! Really useful stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants