Skip to content

Add option to specify multiple threshold values for each threshold types #385

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

Conversation

arun-subramanian-1
Copy link

@arun-subramanian-1 arun-subramanian-1 commented Apr 27, 2019

  • For single threshold value behavior will be same as present, i.e same
    threshold values is used to check against specified threshold types.
    Otherwise parse the value in the same order as the threshold type.

Usage
(1). /p:Threshold="80,100,70" /p:ThresholdType="line,branch,method"
(2). /p:Threshold=90 /p:ThresholdType="line,branch,method"
(3). /p:Threshold="80,90" /p:ThresholdType="branch,method"

closes #379

…pes.

+ For single threshold value behavior will be same as present, i.e same
threshold values is used to check against specified threshold types.
   Otherwise parse the value in the same order as the threshold type.

Usage
(1). /p:Threshold=\"80,100,70\" /p:ThresholdType=\"line,branch,method\"
(2). /p:Threshold=90 /p:ThresholdType=\"line,branch,method\"
(3). /p:Threshold=\"80,90\" /p:ThresholdType=\"branch,method\"
@arun-subramanian-1
Copy link
Author

#379

@codecov
Copy link

codecov bot commented Apr 28, 2019

Codecov Report

Merging #385 into master will increase coverage by 4.36%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #385      +/-   ##
=========================================
+ Coverage   87.13%   91.5%   +4.36%     
=========================================
  Files          16      16              
  Lines        2106    2106              
=========================================
+ Hits         1835    1927      +92     
+ Misses        271     179      -92

@codecov
Copy link

codecov bot commented Apr 28, 2019

Codecov Report

Merging #385 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #385   +/-   ##
=======================================
  Coverage   91.96%   91.96%           
=======================================
  Files          16       16           
  Lines        2229     2229           
=======================================
  Hits         2050     2050           
  Misses        179      179

Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli left a comment

Choose a reason for hiding this comment

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

LGTM if @tonerdo approve we should update guide.

@tonerdo
Copy link
Collaborator

tonerdo commented May 1, 2019

@thetoxicsoul what happens if I specify:
(1). /p:Threshold="80,100,70" /p:ThresholdType="line,branch"
(2). /p:Threshold="80,90" /p:ThresholdType="line,branch,method"

@arun-subramanian-1
Copy link
Author

@tonerdo It will throw a build error like

(1)
error : Threshold type flag count (2) and values count (3) doesnt match

(2)
error : Threshold type flag count (3) and values count (2) doesnt match

@tonerdo
Copy link
Collaborator

tonerdo commented May 12, 2019

I'm concerned that this addition adds some flexibility at the expense of increasing complexity of usage of this relatively simple option

@arun-subramanian-1
Copy link
Author

For our crossplatform .net project we would need this requirement (other windows C++ project we use similar threshold metric).
The only other option would be to use the lowest threshold value for all coverage metric.
This doesnt affect the existing behavior though when user passes the single threshold value.

@MarcoRossignoli
Copy link
Collaborator

other windows C++ project we use similar threshold metric

@arun-subramanian-1 can you provide names of other coverage frameworks with this metrics checks support?

@MarcoRossignoli
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MarcoRossignoli MarcoRossignoli added the enhancement General enhancement request label Jun 11, 2019
@MarcoRossignoli
Copy link
Collaborator

@tonerdo what do you think about this PR?
Should we close and wait for more users request?

@tonerdo
Copy link
Collaborator

tonerdo commented Jul 28, 2019

@MarcoRossignoli I go with the second one. Let's put it on hold till we have more requests

@MarcoRossignoli
Copy link
Collaborator

thank's @arun-subramanian-1 for the effort here...we'll re-open/merge in future if we'll have more feature requests!

@vraravam
Copy link

vraravam commented Mar 2, 2020

I like this feature - could you please re-open and merge the PR?

@MarcoRossignoli
Copy link
Collaborator

@vraravam this PR needs some updated and also we should update the documentation accordingly.

@MadisonEhlers-Vertex
Copy link

Hey all,

This feature would be useful to me as well. Would be great if this feature could be added. Thanks!

@MarcoRossignoli
Copy link
Collaborator

@arun-subramanian-1 I see some interest for this feature, would you like to rebase and move to the end?
If the default behavior is the the same I think we can merge.
Also you should update the guide.

Ah not obligate at all!If you're busy let me know I'll clone your branch and implement the missing part.

@pbmiguel
Copy link
Contributor

@MarcoRossignoli, can I continue this MR - rebase, resolve conflicts?
(this feature would be very helpful to me and my colleagues in my work)

@FranciscoSousaDeveloper

Hey all,

This feature would be useful to me as well. Would be great if this feature could be added. Thanks!

@MarcoRossignoli
Copy link
Collaborator

@MarcoRossignoli, can I continue this MR - rebase, resolve conflicts?

@pbmiguel sorry I missed your message, yep you can go on with rebase.

@pbmiguel
Copy link
Contributor

pbmiguel commented Mar 9, 2021

on it

@pbmiguel
Copy link
Contributor

we had to open a new MR since we can't update the branch of @arun-subramanian-1
#1123

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancement request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specifying threshold level for each threshold type.
7 participants