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

Implement core::fmt::Step for UInt #30

Merged
merged 7 commits into from
Jul 14, 2023
Merged

Conversation

pickx
Copy link
Contributor

@pickx pickx commented Jul 13, 2023

Closes #29.

@danlehmann
Copy link
Owner

danlehmann commented Jul 13, 2023

This looks great, thanks a lot for the contribution.

  1. We should have tests to ensure both the arbitrary int overflowing as well as the underlying type overflowing. I wrote them up (and they pass just fine):
#[cfg(feature = "step_trait")]
#[test]
fn forward_out_of_range() {
    // In range
    assert_eq!(Some(u7::new(121)), Step::forward_checked(u7::new(120), 1));
    assert_eq!(Some(u7::new(127)), Step::forward_checked(u7::new(120), 7));

    // Out of range
    assert_eq!(None, Step::forward_checked(u7::new(120), 8));

    // Out of range for the underlying type
    assert_eq!(None, Step::forward_checked(u7::new(120), 140));
}

#[cfg(feature = "step_trait")]
#[test]
fn backward_out_of_range() {
    // In range
    assert_eq!(Some(u7::new(1)), Step::backward_checked(u7::new(10), 9));
    assert_eq!(Some(u7::new(0)), Step::backward_checked(u7::new(10), 10));

    // Out of range
    assert_eq!(None, Step::backward_checked(u7::new(10), 11));
}

@danlehmann
Copy link
Owner

  1. Github's test runner won't currently test this, as the feature needs to be enabled. We can add that:

In .github/workflows, make a copy of test-const.yml and swap out the feature.

Once that is added and uploaded, Github should automatically run the test suite with this feature enabled.

@danlehmann
Copy link
Owner

Oh! And:

  1. Add the new feature to the CHANGELOG.md, putting it under a new section 1.2.7

@pickx
Copy link
Contributor Author

pickx commented Jul 14, 2023

thanks for the guidance! please let me know if there's anything else I'm missing

@danlehmann
Copy link
Owner

Thanks a lot for your contribution!

@danlehmann danlehmann merged commit 6aefcc8 into danlehmann:main Jul 14, 2023
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.

Range iterators?
2 participants