-
Notifications
You must be signed in to change notification settings - Fork 13
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
add AffectsOverallServiceLevels field to Scorecard and ScorecardInput #282
add AffectsOverallServiceLevels field to Scorecard and ScorecardInput #282
Conversation
Codecov Report
@@ Coverage Diff @@
## main #282 +/- ##
==========================================
- Coverage 77.45% 77.43% -0.02%
==========================================
Files 50 50
Lines 3335 3333 -2
==========================================
- Hits 2583 2581 -2
Misses 549 549
Partials 203 203
|
p := new(bool) | ||
*p = v | ||
return p | ||
return &v |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused about this change and the function in general, doesn't this have a different result than the comment above this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@taimoor-at-opslevel So the problem is that you cannot do this
input := ScorecardInput{
AffectsOverallServiceLevels: &true
}
So you have two ways to fix it - pass the bool value through a function to get it stored in a var and then return the address of the variable
input := ScorecardInput{
AffectsOverallServiceLevels: ol.Bool(true)
}
OR make a local var and give the address
myBool := true
input := ScorecardInput{
AffectsOverallServiceLevels: &myBool
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way the function was originally written was unnecessary - its creates an unneeded pointer - the way David fixed it is more idomatic go.
But you are right we should probably adjust the function docstring
Gonna merge this and fixup the doc string later |
Issues
#71
Changelog
Add
AffectsOverallServiceLevels
field toScorecard
andScorecardInput
structschangie
entryTophatting
task test
passes all tests