-
Notifications
You must be signed in to change notification settings - Fork 312
Enabled doc comments in azure_core and azure_identity packages #2990
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
Conversation
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.
Pull Request Overview
This PR enables documentation warnings in the azure_core
and azure_identity
packages and removes a significant number of unused HTTP headers from azure_core
. The changes aim to improve code documentation consistency and eliminate dead code.
Key changes include:
- Added
#![warn(missing_docs)]
directive to enforce documentation standards - Added comprehensive documentation to numerous public types, methods and fields
- Simplified HTTP header constants by removing unused ones
- Made struct fields public and removed accessor methods in favor of direct field access
- Added documentation comments to macro-generated content
Reviewed Changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
sdk/typespec/typespec_client_core/src/http/request/options/mod.rs | Added documentation to APPLICATION_JSON variant in ContentType macro |
sdk/typespec/typespec_client_core/src/http/request/options/macros.rs | Enhanced macro to support documentation attributes for variants |
sdk/identity/azure_identity/src/lib.rs | Added missing_docs warning directive |
sdk/identity/azure_identity/src/managed_identity_credential.rs | Added comprehensive documentation to public types and methods |
sdk/identity/azure_identity/src/developer_tools_credential.rs | Added method documentation and changed test helper visibility |
sdk/identity/azure_identity/src/client_secret_credential.rs | Added detailed documentation for credential creation |
sdk/identity/azure_identity/src/azure_pipelines_credential.rs | Added struct documentation |
Multiple AMQP and messaging files | Changed from accessor methods to direct field access for better ergonomics |
sdk/core/azure_core/src/lib.rs | Added missing_docs warning and documentation |
sdk/core/azure_core/src/http/request/options/mod.rs | Significantly simplified by removing unused HTTP header constants |
sdk/core/azure_core_*/src/lib.rs | Added missing_docs warnings and comprehensive documentation |
sdk/typespec/typespec_client_core/src/http/request/options/macros.rs
Outdated
Show resolved
Hide resolved
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.
There were a few other instances of warn
that should be expect
so we remember to clean it up in the future and not allow subsequent violations.
Mainly holding because this changes azure_core
and we need to wait until after the service crates ship next week.
Also, be sure to update the sdk/core/azure_core/CHANGELOG.md
file. With as many headers as you removed, you may not have to be quite so specific since we haven't even GA'd yet. Maybe even just "Remove a lot of unused headers." in Breaking Changes
would be fine.
Marked as Draft until Core versions are updated. |
Also:
Removed a significant number of unused HTTP headers from azure_core.