-
Notifications
You must be signed in to change notification settings - Fork 167
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
refactor(crds)!: consistently use message field instead of error field #1542
Conversation
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
✅ Deploy Preview for docs-kargo-akuity-io ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1542 +/- ##
=======================================
Coverage 30.05% 30.05%
=======================================
Files 194 194
Lines 20072 20072
=======================================
Hits 6033 6033
Misses 13781 13781
Partials 258 258 ☔ View full report in Codecov by Sentry. |
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.
Without taking the details of the UI into account, this looks good to me.
💯 for consistency.
@jessesuen and I agreed quite some time ago that status subresources would use
Message
fields instead ofError
fields to preserve the potential for communicating things that are not strictly errors.We have, since then, consistently used
Message
in new status subresource types, but Stages and Warehouses were still usingError
.This was a low-hanging consistency improvement that I wanted to make before breaking changes become more difficult.
cc @rbreeze or @rpelczar, although there are open issues for surfacing info from these fields, I can not find any place in the UI where the field names that are changing are currently referenced. You might want to double-check me.
There are no other changes in this PR.