Skip to content

WIP: New usage and addition fixes #122

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

Closed
wants to merge 8 commits into from
Closed

WIP: New usage and addition fixes #122

wants to merge 8 commits into from

Conversation

pbrisbin
Copy link
Contributor

This is based off of #121. I have a few nits I want to address in that before
prepping the release, which I'll do here. First thing is to use the new usage
ourselves and report coverage with ourselves as a form of integration test.

/cc @mrb

mrb and others added 7 commits September 9, 2016 17:15
Due to a history of legacy functionality changes with the reporter and
on codeclimate.com, this reporter had one major flaw that because of
the circumstances didn't really bother anyone: if tests failed or errors
occurred during a test run, the reporter would still report coverage
to codeclimate.com. This has started to become a problem since we have
released a browser extension and other features which leverage test reports
on non-master or 'default' branches.

The solution is to make this reporter similar to the others for the major
languages we support. Instead of including a snippet in a test/spec helper
which initiates and runs SimpleCov and then posts results to codeclimate,
users will now do what they did before: include the SimpleCov snippet in
their helpers. After tests, users will execute the added bin/codeclimate-ruby
binary, which will aggregate and format results before posting them to
codeclimate. This obviates the "partial results" problem because your CI
script will not reach the line where you run bin/codeclimate-ruby if there
is an error or failure.

We will now have parity between how this reporter is used and how others
are used, and the presenting issues should be mitigated.
Mixed concerns made specs brittle and failure prone
@pbrisbin
Copy link
Contributor Author

@mrb do you plan to merge this directly or into #121?

@pbrisbin
Copy link
Contributor Author

I'm thinking, you should just merge this into #121, get it green, then I'll review it there.

@mrb
Copy link
Contributor

mrb commented Sep 21, 2016

@pbrisbin Okey dokey sounds good!

@pbrisbin
Copy link
Contributor Author

Closing in favor of #121

@pbrisbin pbrisbin closed this Sep 21, 2016
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.

2 participants