Skip to content
This repository was archived by the owner on Oct 2, 2023. It is now read-only.

Conversation

trdarr
Copy link
Contributor

@trdarr trdarr commented Apr 3, 2014

🎤 Check, 1, 2, 3…

@smholloway
Copy link
Member

So this will allow us to see our test coverage at https://coveralls.io/r/toopher? I like it.

@smholloway
Copy link
Member

Still interested in this idea?

@egrim
Copy link
Member

egrim commented Apr 14, 2014

Are you asking someone specific? If it's me: I haven't followed this so
I'm afraid I don't know if I'm interested or not.

On Sun, Apr 13, 2014 at 6:13 PM, Seth Holloway [email protected]:

Still interested in this idea?


Reply to this email directly or view it on GitHubhttps://github.com//pull/28#issuecomment-40323224
.

@trdarr
Copy link
Contributor Author

trdarr commented Apr 14, 2014

@egrim

Are you asking someone specific?

I assume he was asking me.

@smholloway

Still interested in this idea?

Yes, but it's not at the top of my to-do list.

@smholloway
Copy link
Member

It appears to me that the work is done and we're ready to merge. So, can we merge this? If not, what is our criteria for done?

@trdarr
Copy link
Contributor Author

trdarr commented Apr 14, 2014

Yeah, what we have works. Do we care about code coverage for the tests?

@smholloway
Copy link
Member

Test coverage seems useful to me. With Coveralls we can more easily tell if someone added code without adding sufficient tests. It's free and relatively easy to integrate 💯

@trdarr
Copy link
Contributor Author

trdarr commented Apr 14, 2014

My comment was ambiguous. We care about code coverage for __init__.py, of course, but do we also care about code coverage for tests.py? That's what Coveralls currently gives us and it's why I was on the fence about this PR being “complete.”

@smholloway
Copy link
Member

What am I missing? Looking at the most recent build I see coverage numbers for toopher/__init__.py.

COVERAGE FILE LINES RELEVANT COVERED MISSED HITS/LINE
96.26% toopher/init.py 274 187 180 7 1.0
97.57% tests.py 445 288 281 7 1.0

Red/green coverage output here: https://coveralls.io/files/168521327

@trdarr
Copy link
Contributor Author

trdarr commented Apr 14, 2014

I didn't want to merge the PR until I was sure that we're okay with coverage for test.py, which that report includes in addition to coverage for __init__.py.

@egrim
Copy link
Member

egrim commented Apr 14, 2014

I don't think the coverage report for tests.py adds much information
(unless you're a test writer, I suppose).

On Mon, Apr 14, 2014 at 1:07 PM, Thomas Darr [email protected]:

I didn't want to merge the PR until I was sure that we're okay with
coverage for test.py, which that report includes in addition to coverage
for init.py.


Reply to this email directly or view it on GitHubhttps://github.com//pull/28#issuecomment-40398369
.

@smholloway
Copy link
Member

So, we need to figure out how to exclude coverage on tests.py? I don't think the information is dangerous--just not generally useful--so I vote to include it and :shipit:

@smholloway
Copy link
Member

The test file is now omitted; see https://coveralls.io/builds/781900. I will merge tomorrow afternoon unless someone objects.

smholloway added a commit that referenced this pull request May 20, 2014
@smholloway smholloway merged commit ee53902 into develop May 20, 2014
@smholloway smholloway deleted the feature/coveralls branch May 20, 2014 21:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants