Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support limit ratios as percents #536

Merged
merged 3 commits into from
May 18, 2017
Merged

Support limit ratios as percents #536

merged 3 commits into from
May 18, 2017

Conversation

mattnelson
Copy link
Contributor

Pull request for #534

@mattnelson
Copy link
Contributor Author

Not sure what caused the build failure it might be transient.

travis

[INFO] Building: it-dump/pom.xml
[INFO] ..FAILED (14.9 s)
[INFO] The build exited with code 1. See /home/travis/build/jacoco/jacoco/jacoco-maven-plugin.test/target/it/it-dump/build.log for details.

local

[INFO] Building: it-dump/pom.xml
[INFO] run script verify.bsh
[INFO] ..SUCCESS (4.6 s)

@Godin
Copy link
Member

Godin commented May 17, 2017

@mattnelson yes - this test is unstable, relaunched

marchof
marchof reviewed May 17, 2017
View reviewed changes
org.jacoco.report/src/org/jacoco/report/check/Limit.java Outdated
this.minimum = minimum == null ? null : new BigDecimal(minimum);
if (minimum == null) {
this.minimum = null;
} else if (isPercent(minimum)) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe the parsing logic in setMinimum() and setMaximum() can go in a common method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied in c39e3d2

marchof
marchof approved these changes May 18, 2017
View reviewed changes
Copy link
Member

@marchof marchof left a comment

Choose a reason for hiding this comment

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

@mattnelson Looks good to me now, thanks!

@marchof
Copy link
Member

marchof commented May 18, 2017

@mattnelson I added documentation for Ant task and updated change log. Can you please check?

@mattnelson
Copy link
Contributor Author

+1 to 129201d

@marchof marchof merged commit 938b0b4 into jacoco:master May 18, 2017
@Godin Godin added this to the 0.8.0 milestone May 18, 2017
@mattnelson mattnelson deleted the percentages branch May 18, 2017 19:50
@mattnelson
Copy link
Contributor Author

Should the version in master be updated to 0.8.0?

https://github.com/jacoco/jacoco/blob/master/pom.xml#L6
https://oss.sonatype.org/content/repositories/snapshots/org/jacoco/jacoco-maven-plugin/

@Godin
Copy link
Member

Godin commented May 18, 2017 •
edited

@mattnelson IMO there is no need to change version of snapshot even if next release most likely will be 0.8.0 instead of 0.7.9.

@jacoco jacoco locked as resolved and limited conversation to collaborators Jan 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marchof marchof marchof approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.8.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mattnelson @Godin @marchof