Skip to content

Refactor status to be a type#137

Open
martialblog wants to merge 3 commits intomainfrom
status-type
Open

Refactor status to be a type#137
martialblog wants to merge 3 commits intomainfrom
status-type

Conversation

@martialblog
Copy link
Member

@martialblog martialblog commented Feb 6, 2026

For the v1 Release we want to introduce a proper State Type for the v1.0.0 - that type can then just implement the Stringer interface and have a NewFromInt/String function.

Fixes #125

This also changes check.ExitRaw to Exit and removes the Exitf.

@martialblog martialblog added this to the v1.0.0 milestone Feb 6, 2026
@martialblog martialblog self-assigned this Feb 6, 2026
status.go Outdated
type Status int

const (
Invalid Status = iota - 1
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about this one. I kinda wanted something to "mean" error/invalid and we use a -1 value in result.WorstState.

Copy link
Member

Choose a reason for hiding this comment

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

That is one problem of the whole monitoring plugins interface, there is no "error" state

Copy link
Member Author

Choose a reason for hiding this comment

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

Any ideas? just use Unknown and add some custom error types?

Copy link
Member

Choose a reason for hiding this comment

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

hm, we could keep an "error" code and map it to "UNKNOWN" at exit, that would be nice from a programer perspective, but also confusing since the end is the same.

Copy link
Member

Choose a reason for hiding this comment

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

hm, or keep the Status pure and force everyone to use a wrapper with an error.

Copy link
Member

@RincewindsHat RincewindsHat left a comment

Choose a reason for hiding this comment

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

Not ideal, but there is only so much one can do under the given circumstances.
In general I think this is an improvement and we should go with it.

@RincewindsHat
Copy link
Member

sorry @martialblog, I originally wanted to just fix the linter things, but it got out of hand.
I did remove the Invalid status in the end, since I think it should not be there at all, specifically not publicly.
The Statuses are there to symbolize certain test results and I tend to prefer to keep them limited to the "official" ones.

In that spirit I also removed the residual relations between Status and int (namely NewStatusFromInt) to discourage people from using the interchangeable.

If all of that was a stupid idea, don't hesitate to revert those commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Util function to transform string states

2 participants

Comments