Skip to content

Conversation

@pine3ree
Copy link
Contributor

Just a matter of taste: i think it looks uglier as a function static var.
Also, in this way it's also available to all Message(s)

Just a matter of taste: i think it looks uglier as a static function var. In this way it's also available to all Message(s)
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 96.601% when pulling 4010eb5 on pine3ree:patch-24 into 30cfe3c on slimphp:3.x.

@akrabat
Copy link
Member

akrabat commented Jun 2, 2016

Why does coverage go down?

@OptimusCrime
Copy link

Looks like an error in coveralls or something. These two files have identical coverage:
https://coveralls.io/builds/6341055/source?filename=Slim%2FDefaultServicesProvider.php
https://coveralls.io/builds/6390776/source?filename=Slim%2FDefaultServicesProvider.php

So does these:
https://coveralls.io/builds/6341055/source?filename=Slim%2FApp.php
https://coveralls.io/builds/6390776/source?filename=Slim%2FApp.php

The first of each is the current coverage in master (the last push) and the last is the coverage for this pull request.

@pine3ree
Copy link
Contributor Author

pine3ree commented Jun 2, 2016

@akrabat I was wondering the same thing :-)

@llvdl
Copy link
Contributor

llvdl commented Jun 9, 2016

The lines of code with the $valid static variable that where in the function withProtocolVersion had code coverage. Those lines have been taken out and the total count of lines with code coverage has decreased because of this (static class variables don't count for code coverage).

@akrabat akrabat added this to the 3.4.3 milestone Jul 26, 2016
@akrabat akrabat merged commit 4010eb5 into slimphp:3.x Jul 26, 2016
akrabat added a commit that referenced this pull request Jul 26, 2016
@akrabat akrabat changed the title move valid protocol versions to a class static var Move valid protocol versions to a class static var Jul 26, 2016
@akrabat akrabat modified the milestones: 3.5.0, 3.4.3 Jul 26, 2016
@pine3ree pine3ree deleted the patch-24 branch July 28, 2016 15:08
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.

5 participants