Add transaction and span outcome#883
Conversation
❕ Build Aborted
Expand to view the summary
Build stats
Test stats 🧪
Steps errors
Expand to view the steps failures> Show only the first 10 steps failures
Log outputExpand to view the last 100 lines of log output
|
mikker
left a comment
There was a problem hiding this comment.
I'm a little bit confused by this approach.
How about something like
class Transaction
SUCCESS = 'success'
FAILURE = 'failure'
# ...
def success!
self.outcome = SUCCESS
end
def failure!
self.outcome = FAILURE
end
endThis ties the details of how to signify the two values into the class itself. That way we don't have to throw around the magic strings success and failure in the whole lib.
|
Could even be something like class Transaction
class Outcome
def initialize(value)
@value = value
end
def self.success
new('success')
end
def self.failure
new('failure')
end
def self.from_status_code(code)
new(code >= 500 ? 'failure' : 'success')
end
end
def success!
self.outcome = Outcome.success
end
def failure!
self.outcome = Outcome.failure
end
endSuch OO, very message passing 😅 |
Co-authored-by: Mikkel Malmberg <mikkel@brnbw.com>
Co-authored-by: Mikkel Malmberg <mikkel@brnbw.com>
mikker
left a comment
There was a problem hiding this comment.
Generally looks good!
We're a bit inconsistent about whether or not we allow users to set the outcome themselves. Sometimes we take it into account, sometimes only when 'success', sometimes not.
Is there a reason we couldn't always let the user set it themselves? What does the spec suggest?
Co-authored-by: Mikkel Malmberg <mikkel@brnbw.com>
Co-authored-by: Mikkel Malmberg <mikkel@brnbw.com>
Co-authored-by: Mikkel Malmberg <mikkel@brnbw.com>
Co-authored-by: Mikkel Malmberg <mikkel@brnbw.com>
|
Originally, I only took into account whether Edit: Also, we have to check if the outcome is already set in |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Steps errors
Expand to view the steps failures
|
Closes #852
Todo:
http_status_codeargument be converted to anIntegeror required to be anIntegerin the above methods?