Skip to content

Commit

Permalink
Adds CI matrix for crate features (#533)
Browse files Browse the repository at this point in the history
This change makes it so we build our crate with all the various
interesting permutations:

* *default* - build with no features set.
* `--all-features` - build with all the bells and whistles.
* `--features X` where `X` is:
  * `ion-hash`
  * `experimental`
  * `ion-hash,experimental`

When building/testing with these permutations, it uncovered a bug in the
`ion-hash` doc tests where the test code depends on an optional feature.
This has been conditionally compiled out, preserving the text of the
doc comment (only runs when the appropriate feature flag is enabled).

Also updates the build of API docs to error out on leaking internal
references.  This change cleans up such documentation comments.

Co-authored-by: Matthew Pope <81593196+popematt@users.noreply.github.com>
  • Loading branch information
almann and popematt committed Apr 26, 2023
1 parent 4872ae8 commit 448b1b2
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 12 deletions.
25 changes: 17 additions & 8 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ jobs:
# meantime, the build only tests Windows Server 2019.
# See https://github.com/amazon-ion/ion-rust/issues/353
os: [ubuntu-latest, windows-2019, macos-latest]
# build and test for different and interesting crate features
features: ['default', 'all', 'ion-hash', 'experimental', 'ion-hash,experimental']
permissions:
checks: write

Expand All @@ -39,22 +41,27 @@ jobs:
toolchain: stable
components: rustfmt, clippy
override: true
- name: Cargo Build
uses: actions-rs/cargo@v1
with:
command: build
args: --verbose --workspace
- name: Cargo Test (default/no features)
if: matrix.features == 'default'
uses: actions-rs/cargo@v1
with:
command: test
args: --verbose --workspace
- name: Cargo Test (all features)
if: matrix.features == 'all'
uses: actions-rs/cargo@v1
with:
command: test
args: --verbose --workspace --all-features
- name: Cargo Test (specific feature)
if: matrix.features != 'default' && matrix.features != 'all'
uses: actions-rs/cargo@v1
with:
command: test
args: --verbose --workspace --features "${{ matrix.features }}"
- name: Rustfmt Check
# We really only need to run this once--ubuntu/all features mode is as good as any
if: matrix.os == 'ubuntu-latest' && matrix.features == 'all'
uses: actions-rs/cargo@v1
with:
command: fmt
Expand All @@ -68,16 +75,18 @@ jobs:
# The clippy check depends on setup steps defined above, but we don't want it to run
# for every OS because it posts its comments to the PR. These `if` checks limit clippy to
# only running on the Linux test. (The choice of OS was arbitrary.)
if: matrix.os == 'ubuntu-latest'
if: matrix.os == 'ubuntu-latest' && matrix.features == 'all'
run: rustup component add clippy
- name: Run Clippy
if: matrix.os == 'ubuntu-latest'
- name: Run Clippy
if: matrix.os == 'ubuntu-latest' && matrix.features == 'all'
uses: actions-rs/clippy-check@v1
with:
# Adding comments to the PR requires the GITHUB_TOKEN secret.
token: ${{ secrets.GITHUB_TOKEN }}
args: --all-features --tests
- name: Rustdoc on Everything
# We really only need to run this once--ubuntu/all features mode is as good as any
if: matrix.os == 'ubuntu-latest' && matrix.features == 'all'
uses: actions-rs/cargo@v1
with:
command: doc
Expand Down
7 changes: 3 additions & 4 deletions src/ion_data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ pub(crate) use ion_ord::IonOrd;
///
/// Equivalence with respect to Ion values means that if two Ion values, `X` and `Y`, are equivalent,
/// they represent the same data and can be substituted for the other without loss of information.
/// (See [`IonEq`] for more information.)
///
/// Some types, such as [`Element`](crate::Element) and [`Value`](crate::element::Value) cannot be
/// used as the key of a map because they adhere to Rust value semantics—these types cannot implement
Expand All @@ -31,11 +30,11 @@ pub(crate) use ion_ord::IonOrd;
/// For other use cases, such as using Ion data as the key of a map or passing Ion data to an
/// algorithm that depends on [`Eq`], you can lift values using [`IonData::from()`].
///
/// Anything that implements [`IonEq`] can be converted into [`IonData`]. [`IonData`] itself does not
/// implement [`IonEq`] so that it is impossible to create, for example, `IonData<IonData<Element>>`.
/// Generally, anything that is treated as Ion data (e.g., [`Element`](crate::Element) and
/// [`Value`](crate::element::Value)), can be converted to [`IonData`].
///
/// [`Hash`] and [`Ord`] are not guaranteed to be implemented for all [`IonData`], but when they are,
/// they are required to be consistent with [`IonEq`] (and [`Eq`]).
/// they are required to be consistent with Ion structural equality (and [`Eq`]).
///
/// __WARNING__—The Ion specification does _not_ define a total ordering over all Ion values. Do
/// not depend on getting any particular result from [Ord]. Use it only as an opaque total ordering
Expand Down
4 changes: 4 additions & 0 deletions src/ion_hash/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,16 @@
//! use ion_rs::result::IonResult;
//! use ion_rs::ion_hash;
//!
//! # // XXX this doc test requires an optional feature--so this makes sure we can still run tests
//! # #[cfg(feature = "sha2")]
//! # fn main() -> IonResult<()> {
//! let elem = Element::read_one(b"\"hello world\"")?;
//! let digest = ion_hash::sha256(&elem);
//! println!("{:?}", digest);
//! # Ok(())
//! # }
//! # #[cfg(not(feature = "sha2"))]
//! # fn main() {}
//! ```

use digest::{self, FixedOutput, Output, Reset, Update};
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![allow(dead_code)]
#![deny(rustdoc::broken_intra_doc_links)]
#![deny(rustdoc::private_intra_doc_links)]
#![deny(rustdoc::bare_urls)]
//! # Reading and writing `Element`s
//!
Expand Down

0 comments on commit 448b1b2

Please sign in to comment.