Skip to content

api: add comments on fields for reference docs#959

Merged
mtardy merged 1 commit into
mainfrom
pr/mtardy/api-docs
May 19, 2023
Merged

api: add comments on fields for reference docs#959
mtardy merged 1 commit into
mainfrom
pr/mtardy/api-docs

Conversation

@mtardy

@mtardy mtardy commented Apr 28, 2023

Copy link
Copy Markdown
Member

Fixes #957.

This PR includes no functional changes. Only comments for autogenerated documentation.
I need your help with a few fields.

@mtardy mtardy added the area/documentation Improvements or additions to documentation label Apr 28, 2023
Comment thread api/v1/tetragon/tetragon.proto Outdated
Comment thread api/v1/tetragon/tetragon.proto Outdated
Comment thread api/v1/tetragon/tetragon.proto Outdated
Comment thread api/v1/tetragon/tetragon.proto Outdated
Comment thread api/v1/tetragon/tetragon.proto Outdated
@mtardy
mtardy force-pushed the pr/mtardy/api-docs branch from 1c4777f to 1173eca Compare April 28, 2023 15:27
@mtardy

mtardy commented May 2, 2023

Copy link
Copy Markdown
Member Author

Thanks @willfindlay, I added some of your API changes suggestion there #486 (comment).

@mtardy
mtardy force-pushed the pr/mtardy/api-docs branch from 1173eca to f4a972a Compare May 2, 2023 10:23
@mtardy
mtardy marked this pull request as ready for review May 2, 2023 10:23
@mtardy
mtardy requested a review from a team as a code owner May 2, 2023 10:23
@mtardy
mtardy requested a review from jrfastab May 2, 2023 10:23

@sharlns sharlns left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Amazing, LGTM. Thank you! :)

// general flow to exec* a new process, however its possible to replace the
// current process with a new process by doing an exec* without a clone. In
// this case the flag will be omitted and the same PID will be used by the
// kernel for both the old process and the newly exec'd process.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This field is somehow for debugging, it contains some values that may not change but others may...

Would be better to add something like:
The flags is for debugging purposes only and should not be relied upon to propagate comprehensive information.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

when you say "add" you mean append this at the end or at the beginning or replacing the whole flag documentation with this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm fine adding it at the beginning so it is clear and we keep current doc that you added, the developers are on their own here... ;-)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

btw have you seen the related discussion on transforming this into an enum:

Ok let's add that so that we merge this doc!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@mtardy
mtardy force-pushed the pr/mtardy/api-docs branch from f4a972a to 46156cc Compare May 16, 2023 15:53
@netlify

netlify Bot commented May 16, 2023

Copy link
Copy Markdown

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit d8384c2
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/64673aa84af7d000087ee936
😎 Deploy Preview https://deploy-preview-959--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@mtardy
mtardy force-pushed the pr/mtardy/api-docs branch 2 times, most recently from 151f576 to effac5a Compare May 16, 2023 16:04
Also remove empty lines in comments because Markdown does not support
table cell with newlines.

Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
@mtardy
mtardy force-pushed the pr/mtardy/api-docs branch from effac5a to d8384c2 Compare May 19, 2023 09:00
@mtardy

mtardy commented May 19, 2023

Copy link
Copy Markdown
Member Author

Check failure is related to dependabot.yaml config format fails and I have an open PR to fix that. Let's merge this.

@mtardy
mtardy merged commit 6011d31 into main May 19, 2023
@mtardy
mtardy deleted the pr/mtardy/api-docs branch May 19, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation Improvements or additions to documentation

4 participants