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

eventsv1 API documentation seems inaccurate #121374

Open
danwinship opened this issue Oct 19, 2023 · 6 comments
Open

eventsv1 API documentation seems inaccurate #121374

danwinship opened this issue Oct 19, 2023 · 6 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@danwinship
Copy link
Contributor

The eventsv1.Event documentation says:

	// reason is why the action was taken. It is human-readable.
	// This field cannot be empty for new Events and it can have at most 128 characters.
	Reason string `json:"reason,omitempty" protobuf:"bytes,7,name=reason"`

The "human-readable" part contradicts everything else I can find (such as the examples in KEP-383), and in particular, corev1.Event.Reason (which is the same field, right?) is "a short, machine understandable string".

The "cannot be empty for new Events" also contradicts the examples in KEP-383, though maybe that's a case of the KEP being out of date with what was eventually agreed on.

I'm not sure if there are other bugs in the documentation as well. It seems that the docs were updated kind of at the last minute while pushing to v1 (#91645 (comment))

/kind bug
/kind documentation
/sig instrumentation

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 19, 2023
@sftim
Copy link
Contributor

sftim commented Oct 20, 2023

It's machine readable, but it should also make sense to humans (please no error code UUIDs etc here).
We could improve the explanation there.

@sftim
Copy link
Contributor

sftim commented Oct 20, 2023

/sig docs

@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Oct 20, 2023
@danwinship
Copy link
Contributor Author

But the Action field explicitly says "It is machine-readable". So the fact that Reason says "It is human-readable" instead implies that it's supposed to be free-form text

@sftim
Copy link
Contributor

sftim commented Oct 25, 2023

That's more one for the philosophers. Can something be both machine-readable and human readable? I'd say Yes.

However, the comments and generated API docs could still make the situation more clear.

@dashpole
Copy link
Contributor

dashpole commented Nov 2, 2023

/assign @logicalhan

@dashpole
Copy link
Contributor

dashpole commented Nov 2, 2023

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

5 participants