Skip to content

Commit aa12b67

Browse files
jmcarpdavid-crespo
andauthored
Clean up API docstrings, remove "IPv6 unsupported" (#9562)
At the moment, some docstrings in the generated docs lack newlines, and string together multiple sentences without punctuation. For example, the `provisioned` field in the `silo_utilization_list` method is a bit garbled: https://docs.oxide.computer/api/silo_utilization_list. > Accounts for resources allocated by in silos like CPU or memory for running > instances and storage for disks and snapshots Note that CPU and memory > resources associated with a stopped instances are not counted here This patch adds the missing newlines and punctuation to make this docstring readable. Note that these changes won't show up in the generated docs until we cut a new release. Note: if this change makes sense, I'll also ask Claude to review all docstrings in this crate for similar issues. cc @david-crespo in case this is relevant to your interests (docs style guide, making Claude do things). --------- Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
1 parent 894c64e commit aa12b67

File tree

7 files changed

+31951
-54
lines changed

7 files changed

+31951
-54
lines changed
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# API Docstring Style Guide
2+
3+
This guide covers public docstrings (`///`) on endpoint handlers in
4+
`nexus/external-api/src/lib.rs` and on structs, fields, and enums in
5+
`nexus/types/src/external_api/`. These docstrings become user-facing API
6+
documentation.
7+
8+
## Capitalization
9+
10+
Start all docstrings with a capital letter.
11+
12+
```rust
13+
// Bad
14+
/// the source of an identity provider metadata descriptor
15+
pub idp_metadata_source: IdpMetadataSource,
16+
17+
// Good
18+
/// The source of an identity provider metadata descriptor.
19+
pub idp_metadata_source: IdpMetadataSource,
20+
```
21+
22+
## Punctuation
23+
24+
Short, label-like descriptions do not need periods. This includes endpoint
25+
summary lines, struct-level descriptions, and brief field annotations. Longer
26+
explanatory text (especially multi-clause or multi-sentence docstrings) should
27+
end sentences with periods.
28+
29+
```rust
30+
// Endpoint summary lines: no period
31+
/// Fetch silo
32+
/// List identity providers for silo
33+
/// Create IP pool
34+
35+
// Struct-level descriptions: no period
36+
/// View of a Silo
37+
/// A collection of resource counts used to describe capacity and utilization
38+
39+
// Short field descriptions: no period
40+
/// Number of virtual CPUs
41+
/// Common identifying metadata
42+
/// Size of blocks in bytes
43+
44+
// Longer explanatory text: use periods
45+
/// Accounts for resources allocated to running instances or storage
46+
/// allocated via disks or snapshots.
47+
///
48+
/// Note that CPU and memory resources associated with stopped instances
49+
/// are not counted here, whereas associated disks will still be counted.
50+
```
51+
52+
## Articles
53+
54+
Omit articles ("a", "an", "the") in endpoint summary lines. Look at existing
55+
endpoints in `nexus/external-api/src/lib.rs` for reference. A few examples of
56+
the different shapes:
57+
58+
```rust
59+
// Bad
60+
/// Fetch a silo
61+
/// Create an IP pool
62+
/// Delete the project
63+
/// List the silos
64+
65+
// Good
66+
/// Fetch silo
67+
/// Create IP pool
68+
/// Delete project
69+
/// List silos
70+
/// Fetch resource utilization for user's current silo
71+
/// List identity providers for silo
72+
/// Add range to IP pool
73+
```
74+
75+
## Paragraph Separation
76+
77+
Separate distinct thoughts or notes with a blank `///` line. This produces
78+
proper paragraph breaks in rendered documentation.
79+
80+
```rust
81+
// Bad - renders as one run-on paragraph
82+
/// A unique, immutable, system-controlled identifier for the token.
83+
/// Note that this ID is not the bearer token itself, which starts with
84+
/// "oxide-token-".
85+
pub id: Uuid,
86+
87+
// Good - renders as two paragraphs
88+
/// A unique, immutable, system-controlled identifier for the token.
89+
///
90+
/// Note that this ID is not the bearer token itself, which starts with
91+
/// "oxide-token-".
92+
pub id: Uuid,
93+
```
94+
95+
## Acronyms and Abbreviations
96+
97+
Use standard casing for acronyms:
98+
- "IdP" (Identity Provider)
99+
- "SP" (Service Provider)
100+
- "SAML", "ID", "IP", "VPC", "CPU", "RAM", "DER"
101+
102+
## Doc Comments vs Regular Comments
103+
104+
Use `///` for public API documentation that users will see. Use `//` for
105+
internal implementation notes that should not appear in generated docs.
106+
107+
```rust
108+
// Bad - this won't appear in API docs
109+
// A list containing the IDs of the secret keys.
110+
pub secrets: Vec<WebhookSecret>,
111+
112+
// Good - this will appear in API docs
113+
/// A list containing the IDs of the secret keys.
114+
pub secrets: Vec<WebhookSecret>,
115+
```
116+
117+
## Scope
118+
119+
These rules apply to:
120+
- Endpoint handler docstrings (`/// Fetch silo`)
121+
- Public struct docstrings (`/// View of a Silo`)
122+
- Public field docstrings (`/// The IP address held by this resource.`)
123+
- Public enum variant docstrings (`/// The sled is currently active.`)
124+
125+
These rules do NOT apply to:
126+
- Private functions or structs
127+
- Internal comments (`//`)
128+
- Module-level documentation (`//!`)
129+
130+
## General
131+
132+
Follow standard English grammatical rules: correct articles ("a" vs "an"),
133+
subject-verb agreement, proper spelling, etc.

nexus/external-api/src/lib.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ api_versions!([
7979
// | date-based version should be at the top of the list.
8080
// v
8181
// (next_yyyymmddnn, IDENT),
82+
(2026021300, STALE_DOCS_AND_PUNCTUATION),
8283
(2026020901, UPDATE_EXTERNAL_SUBNET_DOCS),
8384
(2026020900, RENAME_POOL_ENDPOINTS),
8485
(2026020600, ADD_SILO_SUBNET_POOLS),
@@ -1152,8 +1153,6 @@ pub trait NexusExternalApi {
11521153
) -> Result<HttpResponseOk<ResultsPage<views::IpPool>>, HttpError>;
11531154

11541155
/// Create IP pool
1155-
///
1156-
/// IPv6 is not yet supported for unicast pools.
11571156
#[endpoint {
11581157
operation_id = "ip_pool_create",
11591158
method = POST,
@@ -1169,8 +1168,6 @@ pub trait NexusExternalApi {
11691168
}
11701169

11711170
/// Create IP pool
1172-
///
1173-
/// IPv6 is not yet supported for unicast pools.
11741171
#[endpoint {
11751172
method = POST,
11761173
path = "/v1/system/ip-pools",
@@ -1502,8 +1499,6 @@ pub trait NexusExternalApi {
15021499

15031500
/// Add range to IP pool
15041501
///
1505-
/// IPv6 ranges are not allowed yet for unicast pools.
1506-
///
15071502
/// For multicast pools, all ranges must be either Any-Source Multicast (ASM)
15081503
/// or Source-Specific Multicast (SSM), but not both. Mixing ASM and SSM
15091504
/// ranges in the same pool is not allowed.
@@ -1527,8 +1522,6 @@ pub trait NexusExternalApi {
15271522

15281523
/// Add range to IP pool
15291524
///
1530-
/// IPv6 ranges are not allowed yet for unicast pools.
1531-
///
15321525
/// For multicast pools, all ranges must be either Any-Source Multicast (ASM)
15331526
/// or Source-Specific Multicast (SSM), but not both. Mixing ASM and SSM
15341527
/// ranges in the same pool is not allowed.

nexus/types/src/external_api/params.rs

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ pub struct SamlIdentityProviderSelector {
242242

243243
// The shape of this selector is slightly different than the others given that
244244
// silos users can only be specified via ID and are automatically provided by
245-
// the environment the user is authetnicated in
245+
// the environment the user is authenticated in
246246
#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq)]
247247
pub struct SshKeySelector {
248248
/// ID of the silo user
@@ -693,9 +693,9 @@ pub struct SiloAuthSettingsUpdate {
693693
/// Create-time parameters for a `User`
694694
#[derive(Clone, Deserialize, JsonSchema)]
695695
pub struct UserCreate {
696-
/// username used to log in
696+
/// Username used to log in
697697
pub external_id: UserId,
698-
/// how to set the user's login password
698+
/// How to set the user's login password
699699
pub password: UserPassword,
700700
}
701701

@@ -792,11 +792,12 @@ pub struct UsernamePasswordCredentials {
792792

793793
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
794794
pub struct DerEncodedKeyPair {
795-
/// request signing public certificate (base64 encoded der file)
795+
/// Request signing public certificate (base64 encoded DER file)
796796
#[serde(deserialize_with = "x509_cert_from_base64_encoded_der")]
797797
pub public_cert: String,
798798

799-
/// request signing RSA private key in PKCS#1 format (base64 encoded der file)
799+
/// Request signing RSA private key in PKCS#1 format
800+
/// (base64 encoded DER file)
800801
#[serde(deserialize_with = "key_from_base64_encoded_der")]
801802
pub private_key: String,
802803
}
@@ -916,25 +917,25 @@ pub struct SamlIdentityProviderCreate {
916917
#[serde(flatten)]
917918
pub identity: IdentityMetadataCreateParams,
918919

919-
/// the source of an identity provider metadata descriptor
920+
/// The source of an identity provider metadata descriptor
920921
pub idp_metadata_source: IdpMetadataSource,
921922

922-
/// idp's entity id
923+
/// IdP's entity ID
923924
pub idp_entity_id: String,
924925

925-
/// sp's client id
926+
/// SP's client ID
926927
pub sp_client_id: String,
927928

928-
/// service provider endpoint where the response will be sent
929+
/// Service provider endpoint where the response will be sent
929930
pub acs_url: String,
930931

931-
/// service provider endpoint where the idp should send log out requests
932+
/// Service provider endpoint where the IdP should send log out requests
932933
pub slo_url: String,
933934

934-
/// customer's technical contact for saml configuration
935+
/// Customer's technical contact for SAML configuration
935936
pub technical_contact_email: String,
936937

937-
/// request signing key pair
938+
/// Request signing key pair
938939
#[serde(default)]
939940
#[serde(deserialize_with = "validate_key_pair")]
940941
pub signing_keypair: Option<DerEncodedKeyPair>,
@@ -1398,7 +1399,7 @@ impl PrivateIpStackCreate {
13981399
/// Create-time parameters for a `Certificate`
13991400
#[derive(Clone, Deserialize, Serialize, JsonSchema)]
14001401
pub struct CertificateCreate {
1401-
/// common identifying metadata
1402+
/// Common identifying metadata
14021403
#[serde(flatten)]
14031404
pub identity: IdentityMetadataCreateParams,
14041405
/// PEM-formatted string containing public certificate chain
@@ -1946,7 +1947,7 @@ pub struct InstanceCreate {
19461947
#[serde(default)]
19471948
pub auto_restart_policy: Option<InstanceAutoRestartPolicy>,
19481949

1949-
/// Anti-Affinity groups which this instance should be added.
1950+
/// Anti-affinity groups to which this instance should be added.
19501951
#[serde(default)]
19511952
pub anti_affinity_groups: Vec<NameOrId>,
19521953

@@ -2317,7 +2318,7 @@ impl JsonSchema for BlockSize {
23172318
schemars::schema::Schema::Object(schemars::schema::SchemaObject {
23182319
metadata: Some(Box::new(schemars::schema::Metadata {
23192320
id: None,
2320-
title: Some("disk block size in bytes".to_string()),
2321+
title: Some("Disk block size in bytes".to_string()),
23212322
..Default::default()
23222323
})),
23232324
instance_type: Some(schemars::schema::InstanceType::Integer.into()),
@@ -2356,7 +2357,7 @@ impl From<DiskVariant> for PhysicalDiskKind {
23562357
pub enum DiskSource {
23572358
/// Create a blank disk
23582359
Blank {
2359-
/// size of blocks for this Disk. valid values are: 512, 2048, or 4096
2360+
/// Size of blocks for this disk. Valid values are: 512, 2048, or 4096.
23602361
block_size: BlockSize,
23612362
},
23622363

@@ -2450,7 +2451,7 @@ pub struct AddressLotCreate {
24502451
pub blocks: Vec<AddressLotBlockCreate>,
24512452
}
24522453

2453-
/// Parameters for creating an address lot block. Fist and last addresses are
2454+
/// Parameters for creating an address lot block. First and last addresses are
24542455
/// inclusive.
24552456
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
24562457
pub struct AddressLotBlockCreate {
@@ -2482,6 +2483,7 @@ pub struct LoopbackAddressCreate {
24822483
pub mask: u8,
24832484

24842485
/// Address is an anycast address.
2486+
///
24852487
/// This allows the address to be assigned to multiple locations simultaneously.
24862488
pub anycast: bool,
24872489
}
@@ -2831,7 +2833,7 @@ pub struct BgpAnnouncementCreate {
28312833
pub network: IpNet,
28322834
}
28332835

2834-
/// Parameters for creating a BGP configuration. This includes and autonomous
2836+
/// Parameters for creating a BGP configuration. This includes an autonomous
28352837
/// system number (ASN) and a virtual routing and forwarding (VRF) identifier.
28362838
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
28372839
pub struct BgpConfigCreate {
@@ -3011,7 +3013,7 @@ pub struct Distribution {
30113013
/// Create-time parameters for an `Image`
30123014
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
30133015
pub struct ImageCreate {
3014-
/// common identifying metadata
3016+
/// Common identifying metadata
30153017
#[serde(flatten)]
30163018
pub identity: IdentityMetadataCreateParams,
30173019

@@ -3030,7 +3032,7 @@ pub struct ImageCreate {
30303032
/// Create-time parameters for a `Snapshot`
30313033
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
30323034
pub struct SnapshotCreate {
3033-
/// common identifying metadata
3035+
/// Common identifying metadata
30343036
#[serde(flatten)]
30353037
pub identity: IdentityMetadataCreateParams,
30363038

nexus/types/src/external_api/shared.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,8 +429,9 @@ pub struct BfdStatus {
429429
pub mode: BfdMode,
430430
}
431431

432-
/// Opaque object representing link state. The contents of this object are not
433-
/// yet stable.
432+
/// Opaque object representing link state.
433+
///
434+
/// The contents of this object are not yet stable.
434435
#[derive(Clone, Debug, Deserialize, Serialize)]
435436
pub struct SwitchLinkState {
436437
link: serde_json::Value,

0 commit comments

Comments
 (0)