Skip to content

#132 clarify when reports are generated and when minimums are applied #133

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

Merged
merged 4 commits into from
Dec 2, 2015

Conversation

RichardBradley
Copy link
Contributor

Fixes #132

@sksamuel
Copy link
Member

Can we split this up as 1.3.2 isn't released yet so updating readme at same time will mean more issues :)

@RichardBradley
Copy link
Contributor Author

Can we split this up as 1.3.2 isn't released yet so updating readme at same time will mean more issues :)

Sure, I've split the branch into two commits - one with the code changes and one with the README and version changes. Is that what you need, or did you want two separate PRs?

I don't think I understand why though...
The PR includes a version number change, so I would expect the workflow to be something like:

  • PR is merged
  • (for a very short time) the README at https://github.com/scoverage/sbt-scoverage displays the new version number (1.3.2) and the new documentation about when reports are generated. Anyone who tries to use 1.3.2 will be disappointed.
  • 1.3.2 is published, either by you or by your automated build systems
  • The README on github is now correct

Did you want to merge this and not publish it or something? Am I just being dumb?

https://travis-ci.org/scoverage/sbt-scoverage/builds/76252063

There was a build failure -- I'll check. This test passes for me locally though. I'll look into it.

@sksamuel
Copy link
Member

Because I can't release until on my proper computer, and I'll get 100
emails guaranteed :)

On 19 August 2015 at 16:38, Richard Bradley [email protected]
wrote:

Can we split this up as 1.3.2 isn't released yet so updating readme at
same time will mean more issues :)

Sure, I've split the branch into two commits - one with the code changes
and one with the README and version changes. Is that what you need, or did
you want two separate PRs?

I don't think I understand why though...
The PR includes a version number change, so I would expect the workflow to
be something like:

  • PR is merged
  • (for a very short time) the README at
    https://github.com/scoverage/sbt-scoverage displays the new version
    number (1.3.2) and the new documentation about when reports are generated.
    Anyone who tries to use 1.3.2 will be disappointed.
  • 1.3.2 is published, either by you or by your automated build systems
  • The README on github is now correct

Did you want to merge this and not publish it or something? Am I just
being dumb?

https://travis-ci.org/scoverage/sbt-scoverage/builds/76252063

There was a build failure -- I'll check. This test passes for me locally
though. I'll look into it.


Reply to this email directly or view it on GitHub
#133 (comment)
.

@sksamuel
Copy link
Member

I guess I can just leave it parked here until I'm ready.

On 19 August 2015 at 16:40, Stephen Samuel (Sam) [email protected] wrote:

Because I can't release until on my proper computer, and I'll get 100
emails guaranteed :)

On 19 August 2015 at 16:38, Richard Bradley [email protected]
wrote:

Can we split this up as 1.3.2 isn't released yet so updating readme at
same time will mean more issues :)

Sure, I've split the branch into two commits - one with the code changes
and one with the README and version changes. Is that what you need, or did
you want two separate PRs?

I don't think I understand why though...
The PR includes a version number change, so I would expect the workflow
to be something like:

  • PR is merged
  • (for a very short time) the README at
    https://github.com/scoverage/sbt-scoverage displays the new version
    number (1.3.2) and the new documentation about when reports are generated.
    Anyone who tries to use 1.3.2 will be disappointed.
  • 1.3.2 is published, either by you or by your automated build systems
  • The README on github is now correct

Did you want to merge this and not publish it or something? Am I just
being dumb?

https://travis-ci.org/scoverage/sbt-scoverage/builds/76252063

There was a build failure -- I'll check. This test passes for me locally
though. I'll look into it.


Reply to this email directly or view it on GitHub
#133 (comment)
.

@RichardBradley
Copy link
Contributor Author

I guess I can just leave it parked here until I'm ready.

That's what I was expecting :-)

@RichardBradley
Copy link
Contributor Author

There was a build failure -- I'll check

I'll come back to this tomorrow or later this week.

@RichardBradley
Copy link
Contributor Author

There was a build failure

Found the problem -- the "scripted" test "bad-coverage" is checking that the coverage minimums are enforced. It ran "sbt clean coverage test" and expected a failure.

I've updated it to run "sbt clean coverage test coverageReport" as per the new design discussed at the top of this thread.

(I didn't know about the "scripted" sbt tests plugin -- that's pretty useful.)

@rorygraves
Copy link
Contributor

@RichardBradley This has a merge conflict, can you please rebase?

@RichardBradley
Copy link
Contributor Author

I've pushed a merge
(Let me know if you'd much rather have a rebase)

@RichardBradley
Copy link
Contributor Author

OK to merge now?

sksamuel added a commit that referenced this pull request Dec 2, 2015
#132 clarify when reports are generated and when minimums are applied
@sksamuel sksamuel merged commit 0bf2087 into scoverage:master Dec 2, 2015
@sksamuel
Copy link
Member

sksamuel commented Dec 2, 2015

Thanks richard

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