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 Extend and From/IntoIterator for SigSet #1553

Merged
merged 1 commit into from Feb 21, 2022

Conversation

magicant
Copy link
Contributor

@magicant magicant commented Oct 6, 2021

No description provided.

@@ -1180,6 +1214,26 @@ mod tests {
}).join().unwrap();
}

#[test]
fn test_extend() {
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference between this and the original test? I don't see any. Could you please move it back to its original location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed to me that existing tests were arranged in the order of implementation, so I reordered this test as I implemented Extend. But if you prefer minimizing the diff, I have no problem doing so. 598f6a6


### Removed

- Removed `SigSet::extend` in favor of `<SigSet as Extend<Signal>>::extend`.
Copy link
Member

Choose a reason for hiding this comment

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

I would put this in the "Changed" section rather than "Removed"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. d1dcaee

@magicant magicant requested a review from asomers October 9, 2021 05:44
@asomers
Copy link
Member

asomers commented Oct 16, 2021

This looks good to me, but you'll have to rebase on master to fix CI, and squash your commits before merging.

@asomers asomers added this to the 0.24.0 milestone Oct 16, 2021
@magicant
Copy link
Contributor Author

Rebased & squashed. Thank you! @asomers

@magicant
Copy link
Contributor Author

Rebased again to resolve the conflict in CHANGELOG.md.

@rtzoeller
Copy link
Collaborator

@magicant can you rebase?

@magicant
Copy link
Contributor Author

Sure. @rtzoeller

@rtzoeller
Copy link
Collaborator

bors r+

@bors bors bot merged commit b0a39cb into nix-rust:master Feb 21, 2022
@magicant magicant deleted the sigset_iterator branch September 11, 2022 14:11
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.

None yet

3 participants