Skip to content

Improve coverage #24#32

Merged
Ocramius merged 14 commits intomasterfrom
improve-coverage-issue-24
Jul 7, 2015
Merged

Improve coverage #24#32
Ocramius merged 14 commits intomasterfrom
improve-coverage-issue-24

Conversation

@asgrim
Copy link
Copy Markdown
Member

@asgrim asgrim commented Jul 3, 2015

Link to issue #24

@asgrim asgrim self-assigned this Jul 3, 2015
@asgrim asgrim added this to the 1.0.0 milestone Jul 3, 2015
@asgrim asgrim mentioned this pull request Jul 3, 2015
6 tasks
@asgrim asgrim force-pushed the improve-coverage-issue-24 branch from f4fcbf8 to 7cbe796 Compare July 3, 2015 13:04
@asgrim asgrim changed the title [WIP] Improve coverage #24 Improve coverage #24 Jul 5, 2015
@asgrim asgrim assigned Ocramius and unassigned asgrim Jul 5, 2015
@asgrim
Copy link
Copy Markdown
Member Author

asgrim commented Jul 5, 2015

Note to @Ocramius: coverage is not complete with this PR, it will just help if we get progress so far merged in. We'll leave #24 open to continue to track coverage improvements

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the reason for this change?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

because it didn't work before, writing the test discovered it did nothing ;)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So what is being done here? Is it just a user-friendly identifier?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, exactly, it turns \BetterReflection\Reflection\ReflectionClass into just ReflectionClass for example

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe rename the method to make that clear?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've removed this method, probably not required anyway

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.

2 participants