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

Provide better documentation for provideDummy/provideDummyBuilder #754

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

Conversation

jamesderlin
Copy link
Contributor

@jamesderlin jamesderlin commented Jun 6, 2024

Mostly fixes #697.

I'm still not quite satisfied with the documentation for provideDummyBuilder since the DummyBuilder typedef remains undocumented, but I can't write documentation for that because I don't understand its arguments.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Mostly fixes dart-lang#697.

I'm still not quite satisfied with the documentation for
`provideDummyBuilder` since the `DummyBuilder` `typedef` remains
undocumented, but I can't write documentation for that because I
don't understand its arguments.
@jamesderlin
Copy link
Contributor Author

@srawlins Could you take a look when you have a chance?

Copy link
Member

@srawlins srawlins left a comment

Choose a reason for hiding this comment

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

Looks great! A few comments

@@ -203,3 +203,71 @@ targets:
```

Also, you can also check out the example configuration in the Mockito repository.


## How do I mock a `sealed` class?
Copy link
Member

Choose a reason for hiding this comment

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

This section discusses how to mock a class that requires instances of sealed classes, rather that mocking sealed classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Heh, I even made a similar correction to a StackOverflow question about this.

I also updated the URLs referring to this section. I feel that the new heading is too long and is more brittle (and I don't think GitHub Markdown supports generating custom slugs?), so I've changed the URLs to be a bit less specific.

FAQ.md Outdated
}
```

and now you want to mock `Foo`. The generated implementation for `MockFoo`
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a bit of a leap from wanting to mock Foo to having a generated implementation. Maybe something like, "A mock implementation of Foo, generated by @GenerateNiceMocks([MockSpec<Foo>()]), needs to ..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do like that better. Fixed.

FAQ.md Outdated
class _FakeBar extends Fake implements Bar {}
```

And then the generated implementation of `MockFoo` can have its `someMethod`
Copy link
Member

Choose a reason for hiding this comment

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

It's not really an "implementation" of MockFoo; you could say the generated declaration of MockFoo or maybe the generated mock implementation of Foo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argh, yeah, I didn't mean "implementation" in the Dart implements sense. I will just remove "the generated implementation of".

@@ -156,7 +157,8 @@ T dummyValue<T>(Object parent, Invocation invocation) {
throw MissingDummyValueError(T);
}

/// Provide a builder for that could create a dummy value of type `T`.
/// Specifies a builder to create a dummy value of type `T`.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks :D

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.

Provide better documentation/examples for provideDummy/provideDummyBuilder
2 participants