Skip to content

Commit

Permalink
fix cargo doc checks in CI (#5924)
Browse files Browse the repository at this point in the history
Fixes #5739.

Also adds `--no-deps` because this is intended as a check to ensure the
docs, if someone wants to generate them, are valid and build without
warnings. Warnings in dependencies' docs are ignored anyway.
  • Loading branch information
iliana authored Jun 21, 2024
1 parent 3549436 commit 92d6530
Show file tree
Hide file tree
Showing 15 changed files with 41 additions and 30 deletions.
6 changes: 6 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
# binaries and the test suite. There's no need for typical library
# documentation of public interfaces.)
#
# NOTE: If you change this, also change the `RUSTDOCFLAGS` values in the various
# CI scripts:
# - .github/buildomat/build-and-test.sh
# - .github/buildomat/jobs/clippy.sh
# - .github/workflows/rust.yml
#
[build]
rustdocflags = "--document-private-items"

Expand Down
2 changes: 1 addition & 1 deletion .github/buildomat/build-and-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ ptime -m bash ./tools/install_builder_prerequisites.sh -y
#
banner build
export RUSTFLAGS="-D warnings"
export RUSTDOCFLAGS="-D warnings"
export RUSTDOCFLAGS="--document-private-items -D warnings"
# When running on illumos we need to pass an additional runpath that is
# usually configured via ".cargo/config" but the `RUSTFLAGS` env variable
# takes precedence. This path contains oxide specific libraries such as
Expand Down
2 changes: 1 addition & 1 deletion .github/buildomat/jobs/clippy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ ptime -m bash ./tools/install_builder_prerequisites.sh -y
banner clippy
export CARGO_INCREMENTAL=0
ptime -m cargo xtask clippy
RUSTDOCFLAGS="-Dwarnings" ptime -m cargo doc --workspace
RUSTDOCFLAGS="--document-private-items -D warnings" ptime -m cargo doc --workspace --no-deps
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,4 @@ jobs:
- name: Install Pre-Requisites
run: ./tools/install_builder_prerequisites.sh -y
- name: Test build documentation
run: RUSTDOCFLAGS="-Dwarnings" cargo doc --workspace
run: RUSTDOCFLAGS="--document-private-items -D warnings" cargo doc --workspace --no-deps
10 changes: 5 additions & 5 deletions nexus/auth/src/authz/api_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
//! Authz types for resources in the API hierarchy
//!
//! The general pattern in Nexus for working with an object is to look it up
//! (see [`crate::db::lookup::LookupPath`]) and get back a so-called `authz`
//! type. This type uniquely identifies the resource regardless of any other
//! changes (e.g., name change or moving it to a different parent collection).
//! The various datastore functions that modify API resources accept these
//! `authz` types.
//! (see `nexus_db_queries::db::lookup::LookupPath`) and get back a so-called
//! `authz` type. This type uniquely identifies the resource regardless of
//! any other changes (e.g., name change or moving it to a different parent
//! collection). The various datastore functions that modify API resources
//! accept these `authz` types.
//!
//! The `authz` types can be passed to
//! [`crate::context::OpContext::authorize()`] to do an authorization check --
Expand Down
4 changes: 2 additions & 2 deletions nexus/db-queries/src/db/datastore/sled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ impl SledTransition {
/// (which is always considered valid).
///
/// For a more descriptive listing of valid transitions, see
/// [`test_sled_transitions`].
/// `test_sled_transitions`.
fn valid_old_policies(&self) -> Vec<SledPolicy> {
use SledPolicy::*;
use SledProvisionPolicy::*;
Expand Down Expand Up @@ -731,7 +731,7 @@ impl SledTransition {
/// (which is always considered valid).
///
/// For a more descriptive listing of valid transitions, see
/// [`test_sled_transitions`].
/// `test_sled_transitions`.
fn valid_old_states(&self) -> Vec<SledState> {
use SledState::*;

Expand Down
2 changes: 1 addition & 1 deletion nexus/db-queries/src/db/datastore/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use omicron_uuid_kinds::TufRepoKind;
use omicron_uuid_kinds::TypedUuid;
use swrite::{swrite, SWrite};

/// The return value of [`DataStore::update_tuf_repo_description_insert`].
/// The return value of [`DataStore::update_tuf_repo_insert`].
///
/// This is similar to [`external::TufRepoInsertResponse`], but uses
/// nexus-db-model's types instead of external types.
Expand Down
13 changes: 7 additions & 6 deletions nexus/reconfigurator/planning/src/blueprint_builder/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1164,12 +1164,13 @@ impl<'a> BlueprintZonesBuilder<'a> {

/// Helper for working with sets of disks on each sled
///
/// Tracking the set of disks is slightly non-trivial because we need to bump
/// the per-sled generation number iff the disks are changed. So we need to
/// keep track of whether we've changed the disks relative to the parent
/// blueprint. We do this by keeping a copy of any [`BlueprintDisksConfig`]
/// that we've changed and a _reference_ to the parent blueprint's disks. This
/// struct makes it easy for callers iterate over the right set of disks.
/// Tracking the set of disks is slightly non-trivial because we need to
/// bump the per-sled generation number iff the disks are changed. So
/// we need to keep track of whether we've changed the disks relative
/// to the parent blueprint. We do this by keeping a copy of any
/// [`BlueprintPhysicalDisksConfig`] that we've changed and a _reference_ to
/// the parent blueprint's disks. This struct makes it easy for callers iterate
/// over the right set of disks.
struct BlueprintDisksBuilder<'a> {
changed_disks: BTreeMap<SledUuid, BlueprintPhysicalDisksConfig>,
parent_disks: &'a BTreeMap<SledUuid, BlueprintPhysicalDisksConfig>,
Expand Down
3 changes: 2 additions & 1 deletion nexus/src/app/instance_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ use std::str::FromStr;
use uuid::Uuid;

use super::background::BackgroundTasks;
use super::Nexus;

impl super::Nexus {
impl Nexus {
/// Returns the set of switches with uplinks configured and boundary
/// services enabled.
pub(crate) async fn boundary_switches(
Expand Down
6 changes: 5 additions & 1 deletion nexus/src/app/sagas/volume_delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,9 @@ async fn svd_delete_crucible_snapshot_records(
/// It's insufficient to rely on the struct of CrucibleResources to clean up
/// that is returned as part of svd_decrease_crucible_resource_count. Imagine a
/// disk that is composed of three regions (a subset of
/// [`VolumeConstructionRequest`] is shown here):
/// [`sled_agent_client::types::VolumeConstructionRequest`] is shown here):
///
/// ```json
/// {
/// "type": "volume",
/// "id": "6b353c87-afac-4ee2-b71a-6fe35fcf9e46",
Expand All @@ -352,9 +353,11 @@ async fn svd_delete_crucible_snapshot_records(
/// ],
/// "read_only_parent": null,
/// }
/// ```
///
/// Taking a snapshot of this will produce the following volume:
///
/// ```json
/// {
/// "type": "volume",
/// "id": "1ef7282e-a3fb-4222-85a8-b16d3fbfd738", <-- new UUID
Expand All @@ -373,6 +376,7 @@ async fn svd_delete_crucible_snapshot_records(
/// ],
/// "read_only_parent": null,
/// }
/// ```
///
/// The snapshot targets will use the same IP but different port: snapshots are
/// initially located on the same filesystem as their region.
Expand Down
2 changes: 1 addition & 1 deletion nexus/types/src/deployment/blueprint_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

//! Types helpful for diffing [`Blueprints`].
//! Types helpful for diffing blueprints.
use super::blueprint_display::{
constants::*, linear_table_modified, linear_table_unchanged, BpDiffState,
Expand Down
3 changes: 1 addition & 2 deletions nexus/types/src/deployment/blueprint_display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

//! Types helpful for rendering [`Blueprints`].
//! Types helpful for rendering blueprints.
use omicron_common::api::external::Generation;
use std::fmt;
Expand Down Expand Up @@ -176,7 +176,6 @@ pub trait BpSledSubtableData {
}

/// A table specific to a sled resource, such as a zone or disk.
/// `BpSledSubtable`s are always nested under [`BpSledTable`]s.
pub struct BpSledSubtable {
table_name: &'static str,
column_names: &'static [&'static str],
Expand Down
4 changes: 2 additions & 2 deletions oximeter/db/src/oxql/ast/grammar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ peg::parser! {
///
/// We support the following common escape sequences:
///
/// ```ignore
/// ```text
/// \n
/// \r
/// \t
Expand All @@ -271,7 +271,7 @@ peg::parser! {
/// styles if required, by writing them as their Unicode escape
/// sequences. For example, this string:
///
/// ```ignore
/// ```text
/// "this string has \u{22} in it"
/// ```
///
Expand Down
10 changes: 5 additions & 5 deletions sled-agent/src/sim/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ enum MonitorChange {
/// integration tests.
///
/// The simulated instance contains a fake instance state stored as a
/// [`propolis_client::api::InstanceStateMonitorResponse`]. Transition requests
/// enqueue changes to either the instance state or the migration status fields
/// of this response. When poked, the simulated instance applies the next
/// transition, translates this to an observed Propolis state, and sends it
/// off for processing.
/// [`propolis_client::types::InstanceStateMonitorResponse`]. Transition
/// requests enqueue changes to either the instance state or the migration
/// status fields of this response. When poked, the simulated instance applies
/// the next transition, translates this to an observed Propolis state, and
/// sends it off for processing.
#[derive(Debug)]
struct SimInstanceInner {
/// The current simulated instance state.
Expand Down
2 changes: 1 addition & 1 deletion wicket/src/ui/panes/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ pub struct UpdatePane {
/// TODO: Move following state into global `State` so that recorder snapshots
/// capture all state.
///
/// TODO: The <usize> generic parameter is carried over from earlier versions
/// TODO: The usize generic parameter is carried over from earlier versions
/// of tui-tree-widget, but there's likely a better index type.
tree_state: TreeState<usize>,
items: Vec<TreeItem<'static, usize>>,
Expand Down

0 comments on commit 92d6530

Please sign in to comment.