elastic / crd-ref-docs

Generates Kubernetes CRD API reference documentation
Apache License 2.0
103 stars 53 forks source link

feat: improve parsing #51

Closed tenstad closed 1 year ago

tenstad commented 1 year ago

Simplifies a lot of the complexity of having separate processType, loadType and loadAliasType.

Removes ArrayKind, as it is not supported by controller-gen.

Also ensures correct parsing of A in the example below, making A.UnderlyingType equal B instead of string. This will be required to correctly parse and propagate markers (https://github.com/elastic/crd-ref-docs/pull/49).

type Foo struct {
    A A `json:"a,omitempty"`
}

type A B
type B string
thbkrkr commented 1 year ago

This is very nice!

Also ensures correct parsing of A in the example below, making A.UnderlyingType equal B instead of string. This will be required to correctly parse and propagate markers (https://github.com/elastic/crd-ref-docs/pull/49).

Could you update test/api/v1/guestbook_types.go with an example that covers this?

Suggestion but feel free to rename:

// UnderlyingType tests that A.UnderlyingType equals B instead of string.
// +kubebuilder:object:root=true
type UnderlyingType struct {
    A A `json:"a,omitempty"`
}

type A B
type B string

I think we need to adapt RenderTypeLink a bit in both renderers because we can now have a link in a title which isn't rendered as expected.

cla-checker-service[bot] commented 1 year ago

💚 CLA has been signed

thbkrkr commented 1 year ago

cla-checker-service is not happy because you used an @ not linked to your GitHub account on the last commit

tenstad commented 1 year ago

Added the test in two commits, where the 2nd (https://github.com/elastic/crd-ref-docs/pull/51/commits/319af4e1cc00149fd723771670fdd70b52fd0c5b) shows the changes introduced by this PR.

tenstad commented 1 year ago

Wondering if we should update the rendering so that the test types we added renders as they used to 🤔

tenstad commented 1 year ago

Having different descriptions, I guess it is nice to show both Underlying1 and Underlying2 :)

thbkrkr commented 1 year ago

Last changes are 👍 👍 .

I think that the last bits to fix are for the rendering.

In asciidoc, it looks like xref doesn't work well when used in a title. See the example below where the cross reference for Underlying2 doesn't work:

Capture d’écran 2023-10-06 à 10 24 51

The only idea I have for now is to use link:#, which seems a bit hacky?

For markdown, we need to add the backquotes conditionally otherwise the link is not rendered:

Capture d’écran 2023-10-06 à 11 57 31
tenstad commented 1 year ago

What if we display Underlying type: in both adoc and markdown. And show the type as in a struct field?