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

Docs: remove Safety notes from functions which are not unsafe; mark get_module_*_conf unsafe #130

Merged
merged 2 commits into from
Feb 14, 2025

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Feb 12, 2025

Proposed changes

This PR removes # Safety sections from the rustdocs of functions which are not unsafe. I noticed this when reading one of the docs and scanned through the entire repo's use of # Safety docs to collect this set.

The Request::get_module_*_conf functions had Safety notes describing their invariants but were not unsafe, despite definitely being unsafe, so I changed those function signatures instead of removing the docs.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have written my commit messages in the Conventional Commits format.
  • I have read the CONTRIBUTING doc
  • I have added tests (when possible) that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@pchickey pchickey changed the title Docs: remove Safety notes from functions which are not unsafe Docs: remove Safety notes from functions which are not unsafe; mark get_module_*_conf unsafe Feb 12, 2025
Copy link
Member

@bavshin-f5 bavshin-f5 left a comment

Choose a reason for hiding this comment

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

Can you please reorganize the changes into 2 commits: Request method changes and safety notes removal.
Otherwise looks good.

@bavshin-f5 bavshin-f5 merged commit c6b8b2e into nginx:master Feb 14, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants