Azure / Azure/azure-sdk-for-rust
[Service Bus] Resolve PR #2863 review debt
- Dominant language
- Rust
- Stars
- 884
- Forks
- 365
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 109
Description
## Summary
PR #2863 landed the initial `azure_messaging_servicebus` crate with a large set of unresolved review comments. This issue tracks those comments. It holds 43 items, and 38 are still open.
## Motivation
The review debt is not evenly spread. Thirteen of the 38 open items are the same argument about where the types live, what the crate exports, how the client is constructed, and which fields are public. That makes the public API shape the gate on the architecture board review, so those items should close before the session receiver, the rule manager, and the processor add more public types.
The items below are grouped by the issue that owns them. Each group closes with its owning issue rather than on its own.
## Proposal
Resolve the items in each group as part of the owning issue, then close this issue when all groups are clear.
### Error type (#4938)
- [ ] Fix ServiceBusError, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2255312629 , https://github.com/Azure/azure-sdk-for-rust/pull/2863/files#r2271554197 , https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271554197 , https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271553225 , https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271551809)
- [ ] Unwrap failures, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258311431)
- [ ] Move error, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271566034)
- [ ] Cleanup Errors, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2255312629)
### Recoverable connection and authorizer (#4940)
- [ ] Explore not holding the connection and session, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258189472)
- [ ] Review the timeouts, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288835060)
- [ ] Authorizer cleanup, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288839200)
### Public API shape (#4941)
- [ ] ReceiveMode copy, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258234837)
- [ ] Don't expose as Hashmap, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258319466)
- [ ] Mutability, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258322826)
- [ ] &str vs &String, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2255420541)
- [ ] Review where types live and are exported, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271562699)
- [ ] Move clients, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271603478)
- [ ] Enable options, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271606333)
- [ ] Review where types live, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2255318367)
- [ ] Cleanup exports, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2286456811)
- [ ] Change the builder pattern, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2286459626)
- [ ] SubClient cleanups, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2286463951)
- [ ] SubQueue surface area, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288819989)
- [ ] Move the builder, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288827077)
### Message model (#4942)
- [ ] Single message struct, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2257841558)
- [ ] AMQP message initilialization, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2257858451)
- [ ] Hashmap consistency, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258184706)
- [ ] Only convert binary, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258315774)
- [ ] Serialization, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271557725)
### Settlement (#4943)
- [ ] Dead letter cleanup, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288817741)
### Core AMQP crate (#4935, #4936)
- [ ] azure_core_amqp consolidation, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2255421514)
### Connection string credentials (#4956)
- [ ] Connection string remnant, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288850082)
### Tests (#4957)
- [ ] Tracing simplication in tests, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258355215)
- [ ] Async test cleanup, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288852970)
- [ ] New resource cleanup, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288851133)
### Documentation and samples (#4959)
- [ ] az usage in crate readme and scripts, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271543930)
- [ ] Cleanup README to clarify where commands run, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271552598)
- [ ] Example boiler plate, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2271609637)
- [ ] Remove or cleanup Contributing.md, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2286449162)
- [ ] Trim the fluff in comments, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2286473109)
### Crate hygiene and constants (#4962)
- [ ] Remove deadcode, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258036898)
- [ ] Add manifest constants, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2258230824)
### Already resolved
- [x] ClientOptions field removal, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2288821560)
- [x] Bicep file cleanup, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2255250470)
- [x] Bicep file move, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2255248979)
- [x] Bicep file cleanup, see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2255254329)
- [x] Files should use '/', see (https://github.com/Azure/azure-sdk-for-rust/pull/2863#discussion_r2286450862)
## Related
Sub-issue of #4934.
The four open sub-issues of this issue are covered by the groups above. #2920 and #2921 belong to the error type group. #2923 belongs to the message model group. #2925 belongs to the recoverable connection group.
Contributor guide
Assessment
This issue has not been assessed yet.