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

Various improvements #47

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mcarton
Copy link
Contributor

@mcarton mcarton commented May 21, 2019

  • Fix review comments from the other PR.
  • Update dependencies.
  • Document matches in rustdoc. While the guide is really good, having a comprehensive list of all matchers can be helpful.

@@ -511,7 +511,7 @@ fn generate_mock_for_traits(
};
generated_items.push(debug_impl_item);

let has_generic_method = Itertools::flatten(traits.iter().map(|&(_, members)| members.iter()))
let has_generic_method = traits.iter().map(|&(_, members)| members.iter()).flat_map(|x| x)
Copy link
Owner

Choose a reason for hiding this comment

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

Why not flatten()?

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 was removed from itertools.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, because it was added to standard library.

doc/guide.md Outdated
@@ -928,7 +928,7 @@ mod tests {

## Error messages

The *Mockers* library tries to produce helpful error messages. Note that error messages are better on a nightly compiler, because the `proc_macro_diagnostic` is not yet stable. It highlights key operations so you can easily spot a problem.
The *Mockers* library tries to produce helpful error messages. It highlights key operations so you can easily spot a problem. Note that error messages are better on a nightly compiler, because the `proc_macro_diagnostic` is not yet stable.
Copy link
Owner

Choose a reason for hiding this comment

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

I dropped this entirely from master because 'errors' here are runtime errors produced by mockers library during test. It has no relation to compile-time errors.

@@ -3,6 +3,7 @@ use proc_macro2::{Span, TokenStream};
use quote::ToTokens;
use std::collections::{HashMap, HashSet};
use std::result::Result;
use std::sync::atomic::AtomicUsize;
Copy link
Owner

Choose a reason for hiding this comment

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

I already have pending commit replacing ID generators with AtomicUsize, and it's little more comprehensive. So please drop this one.

@@ -21,6 +26,19 @@ impl<T: Debug, M: MatchArg<T>> MatchArg<Option<T>> for MatchSome<T, M> {
format!("some({})", self.0.describe())
}
}

/// Matches a result for some value matching another matcher.
Copy link
Owner

Choose a reason for hiding this comment

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

"Matches value inside Option with another matcher"?

mockers/Cargo.toml Show resolved Hide resolved
$satisfy:ident, $satisfy_clone:ident,
$expectation:ident, $expectation_times:ident)
{ $(($n:tt, $arg:ident, $Arg:ident)),* }
$callmatch:ident, $actionclone:ident,
Copy link
Owner

Choose a reason for hiding this comment

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

No need to rename macro params.

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

2 participants