-
Notifications
You must be signed in to change notification settings - Fork 712
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
Fixes broken link #4060
Fixes broken link #4060
Conversation
// https://github.com/awslabs/s2n/blob/main/docs/USAGE-GUIDE.md#s2n_connection_set_send_cb | ||
// https://aws.github.io/s2n-tls/doxygen/s2n_8h.html#a699fd9e05a8e8163811db6cab01af973 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the link to the correct section right now-- will it always be? Is this a predictable hash, or is it random / based on the order of methods in the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to this issue the hash won't change unless we change the definition of the function. So this link won't break unless the API changes, which is unlikely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this is overkill, but we could probably get the domain for something like docs.s2n-tls.aws.dev
and then CNAME that to the doxygen site. This would also let get more human readable names for the things that host our CloudFront distributions with coverage reports, benchmark reports, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a hash is it possible to generate nicer human readable names for the url?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not possible to do that with the current doxygen setup. The links are hashes on purpose, as the docs mention, because cpp can overload function names and therefore it wouldn't be clear which function the link was referring to.
Resolved issues:
resolves #4054
Description of changes:
The URL sucks but it is indeed a link to the correct doxygen section
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.