search

metal3-io / baremetal-operator

Bare metal host provisioning integration for Kubernetes
Apache License 2.0
570 stars 253 forks source link

⚠️ Migrate information away from docs/api.md #1820

Closed dtantsur closed 1 month ago

dtantsur commented 3 months ago

The motivation of this change is to avoid duplicating information between the inline API docs, the user guide and this page.

Some useful bits of information have been migrated to the inline docs, which I also proof-read for mistakes. Then most of the content of api.md has been replaced with references to the rendered CR docs and the user guide.

Signed-off-by: Dmitry Tantsur dtantsur@protonmail.com

Rozzii commented 2 months ago

I would like you to please mention the apis/metal3.io/v1alpha1/baremetalhost_types.go in the new api.md just to make it more easily discoverable, there are info now in that file that might help someone better understand the BMO's features. Otherwise LGTM.

dtantsur commented 2 months ago

I would like you to please mention the apis/metal3.io/v1alpha1/baremetalhost_types.go in the new api.md just to make it more easily discoverable

Aren't the already provided links to the rendered CRD docs better? Especially given that

inline comments are not documentation!

mboukhalfa commented 2 months ago

Aren't the already provided links to the rendered CRD docs better?

We can discuss that if we assume the user has bmo running before checking the API docs

dtantsur commented 2 months ago

We can discuss that if we assume the user has bmo running before checking the API docs

I'm not sure what you're talking about. Could you please double-check the changes to docs/api.md here? I'm adding links to https://doc.crds.dev/github.com/metal3-io/baremetal-operator/, is it something you don't consider acceptable?

In any case, having the same information in 2 places is a recipe for getting outdated information.

mboukhalfa commented 2 months ago

https://doc.crds.dev/github.com/metal3-io/baremetal-operator/

That a good idea I was thinking CRDs docs rendered with kubectl

dtantsur commented 2 months ago

@Rozzii @mboukhalfa could you re-review given what we've discussed? I believe your requests are already addressed in a slightly better way.

mboukhalfa commented 2 months ago

@Rozzii @mboukhalfa could you re-review given what we've discussed? I believe your requests are already addressed in a slightly better way.

I will check a bit later in details and might give some feedback I think I do not have the right to lgtm on this repo, I was wondering where the examples like HostFirmwareSettings Example got migrated to ?

dtantsur commented 2 months ago

where the examples like HostFirmwareSettings Example got migrated to ?

https://book.metal3.io/bmo/firmware_settings

Rozzii commented 2 months ago

I would like you to please mention the apis/metal3.io/v1alpha1/baremetalhost_types.go in the new api.md just to make it more easily discoverable

Aren't the already provided links to the rendered CRD docs better? Especially given that

inline comments are not documentation!

I meant mentioning the apis/metal3.io/v1alpha1/baremetalhost_types.go in addition to the auto generated CRD docs and the book. I know it overlaps with the CRDs but IMO it still have info that is not present on the auto generated CRDs e.g. https://github.com/metal3-io/baremetal-operator/pull/1820/files#diff-0f3e356f6156566888fdbc0e7c0fa713ee054933106059fc29f702f8d1a554f2R365-R426

Like the software and hardware raid are mutually exclusive or that the hostFirmwareSetting CR is an alternative to the BMH firmware field. Just tiny bits of info that might help someone connect the dots better.

My request is just to simply put a line in the api.md that say something like this "Additional API implementation details can be found in apis/metal3.io/v1alpha1/baremetalhost_types.go " , just to let the newcomers know that we hove some info bits there too.

dtantsur commented 2 months ago

Like the software and hardware raid are mutually exclusive or that the hostFirmwareSetting CR is an alternative to the BMH firmware field. Just tiny bits of info that might help someone connect the dots better.

This is a case for a user guide IMO.

My request is just to simply put a line in the api.md that say something like this "Additional API implementation details can be found in apis/metal3.io/v1alpha1/baremetalhost_types.go "

Will do

metal3-io-bot commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kashifest, mboukhalfa

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)~~ [kashifest] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment