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

Delete internal API docs after generation #10065

Merged
merged 16 commits into from
Nov 15, 2024

Conversation

sheidkamp
Copy link

@sheidkamp sheidkamp commented Nov 14, 2024

Description

Delete external API docs after generation

Docs changes

Delete the generated internal API docs after generation

May require a manual update of existing files in firebase Confirmed with docs that it does not.

Testing steps

Run make go-generate-apis fmt

validate that the removed files are no longer present on the file-system:

ls -l docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/external/
ls: docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/external/: No such file or directory

Notes for reviewers
There isn't a good way to validate this e2e/in CI without running against production, so we will need to run this through ci after review merge, ideally with k8sgateway#10334 which needs the same validation.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

@sheidkamp sheidkamp requested a review from a team as a code owner November 14, 2024 15:32
@solo-changelog-bot
Copy link

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/6768

@nfuden nfuden changed the title Remove external API docs [Migrated] Add CVE-2024-45258 to trivyignore Nov 14, 2024
@nfuden nfuden closed this Nov 14, 2024
@sheidkamp sheidkamp reopened this Nov 14, 2024
@sheidkamp sheidkamp changed the title [Migrated] Add CVE-2024-45258 to trivyignore Delete external API docs after generation Nov 14, 2024
Copy link

@Rachael-Graham Rachael-Graham left a comment

Choose a reason for hiding this comment

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

Reposting from Slack for visibility -

In several of our gloo api docs we link to a bunch of these pages, so many links will be broken if we delete these.
I had a hunch that we autogenerate some of these links. for example, in the settings.proto reference docs, we link to the consul query_options proto (which is one of the files being deleted). but in the actual settings proto we don’t hardcode a link.

in solo-kit it looks like the API ref doc template uses a linkForField function. the code for that function is here.

long story short - i think we would need to change that linkForField function in solo-kit to somehow recognize when the field is one of these external protos that we will no longer be creating docs for, and either simply do not create a link for it, or hard-code a link to the appropriate external doc (like a link to envoy api docs etc)

@sheidkamp sheidkamp changed the title Delete external API docs after generation Delete internal API docs after generation Nov 15, 2024
Copy link

@Rachael-Graham Rachael-Graham left a comment

Choose a reason for hiding this comment

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

Thank you!!

@sheidkamp sheidkamp closed this Nov 15, 2024
@sheidkamp sheidkamp reopened this Nov 15, 2024
@sheidkamp sheidkamp merged commit 39972ee into main Nov 15, 2024
17 of 18 checks passed
@sheidkamp sheidkamp deleted the sheidkamp/remove-external-api-docs branch November 15, 2024 20:50
sheidkamp added a commit that referenced this pull request Nov 18, 2024
…docs

Delete internal API docs after generation
sheidkamp added a commit that referenced this pull request Nov 18, 2024
…docs

Delete internal API docs after generation
sheidkamp added a commit that referenced this pull request Nov 18, 2024
…docs

Delete internal API docs after generation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants