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.

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

Standardize values and type of outcome #238

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

xbcsmith
Copy link

@xbcsmith xbcsmith commented Jul 8, 2024

Changes

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

@xbcsmith xbcsmith requested a review from a team as a code owner July 8, 2024 15:50
Signed-off-by: Brett Smith <bc.smith@sas.com>
@e-backmark-ericsson
Copy link
Contributor

See also #215

Copy link
Contributor

@xibz xibz left a comment

Choose a reason for hiding this comment

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

@afrittoli - I think this change is fine, but you had some comments from #237, but I think this PR still satisfies your concerns. I think this approach is just standardizing on what word to use to signify passing or failing of a test, in this case success and failure given that other enums rely on success and failure.

@xibz
Copy link
Contributor

xibz commented Nov 11, 2024

This does get rid of cancel. Should that be a value that we still need, though?

Copy link
Contributor

@xibz xibz left a comment

Choose a reason for hiding this comment

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

@olensmar - Hey Ole, we are interested in enforcing some consistency across events.

So the idea is

  • pass -> success
  • fail -> failure

While pass is pretty standard to testing frameworks, I'd be okay keeping it as passed, but if we want to use fail, we should decide whether we use fail everywhere or failure. Since failure is used more widely, we opted to use failure. How do you feel about this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants