Skip to content
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

Put Ion 1.1 support behind a feature flag #842

Merged
merged 2 commits into from
Oct 16, 2024
Merged

Conversation

popematt
Copy link
Contributor

Issue #, if available:

Description of changes:

I tried to make this as minimally invasive as possible:

  • The experimental-ion-1-1 feature enablesexperimental-reader-writer
  • The experimental feature flag enables experimental-ion-1-1
  • When experimental-ion-1-1 is not enabled
    • the v1_1 module is only pub(crate)
    • the decoder doesn't have a match arm for the (1, 1) version.
    • the tests for switching versions have a branch that checks that an Err is returned for an Ion 1.1 IVM
    • the conformance dsl tests and a number of Ion 1.1 specific tests are disabled (omitted from compilation, really)
  • When experimental-ion-1-1 is enabled, all APIs and behaviors are as they were prior to this PR

I also updated the build workflow to not cancel all outstanding jobs if one job in the build matrix fails. It's a lot quicker to identify issues if you can see that e.g. the tests pass for certain feature sets but not for others.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -1860,6 +1860,11 @@ mod tests {

expect_int(context_ref, &mut reader, IonEncoding::Text_1_0, 2)?;

if cfg!(not(feature = "experimental-ion-1-1")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL about the cfg macro.

@@ -125,6 +125,7 @@ pub trait RawVersionMarker<'top>: Debug + Copy + Clone + HasSpan<'top> {
fn stream_version_after_marker(&self) -> IonResult<IonVersion> {
match self.major_minor() {
(1, 0) => Ok(IonVersion::v1_0),
#[cfg(feature = "experimental-ion-1-1")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL you could apply #[cfg(...)] annotations to a match arm.

@popematt popematt merged commit 05ba5e8 into amazon-ion:main Oct 16, 2024
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants