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

Adds some documentation/examples #51

Merged
merged 10 commits into from Jun 23, 2020
Merged

Conversation

kritzcreek
Copy link
Contributor

Currently making a pass over some of the modules until I run out of steam...

src/Iter.mo Show resolved Hide resolved
Co-authored-by: Joachim Breitner <mail@joachim-breitner.de>
@kritzcreek kritzcreek marked this pull request as ready for review June 22, 2020 15:02
src/Iter.mo Show resolved Hide resolved
src/Iter.mo Show resolved Hide resolved
src/Iter.mo Outdated Show resolved Hide resolved
src/Iter.mo Show resolved Hide resolved
src/Iter.mo Outdated Show resolved Hide resolved
src/Iter.mo Show resolved Hide resolved
src/Iter.mo Outdated Show resolved Hide resolved
src/Option.mo Outdated Show resolved Hide resolved
src/Option.mo Outdated Show resolved Hide resolved
src/Result.mo Outdated
Comment on lines 9 to 10
/// Motoko does not have exceptions, so we use a datatype to encode these
/// outcomes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Motoko does not have exceptions, so we use a datatype to encode these
/// outcomes.
/// Motoko does not have exceptions, so we use a datatype to encode these
/// outcomes.
/// Motoko's exception handling is limited to `async` contexts.
/// For other contexts, Motoko provides a variant type to encode exceptional results.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of making that rather confusing statement, maybe just delete the reference to exceptions, and justify the existence of Result otherwise?

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 agree we shouldn't be bringing exceptions into this, maybe we should say something about error handling in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allright, I basically copied https://doc.rust-lang.org/std/result/ because I thought the description was on point.

src/Result.mo Outdated Show resolved Hide resolved
src/Result.mo Outdated Show resolved Hide resolved
src/Result.mo Outdated Show resolved Hide resolved
src/Result.mo Show resolved Hide resolved
src/Result.mo Show resolved Hide resolved
@kritzcreek
Copy link
Contributor Author

Thanks for all the comments and review, I'll merge this and if we think the Result introduction needs more improvement we can still do that.

@kritzcreek kritzcreek merged commit 44470a9 into master Jun 23, 2020
@kritzcreek kritzcreek deleted the christoph/documentation branch June 23, 2020 08:59
Copy link
Contributor

@matthewhammer matthewhammer left a comment

Choose a reason for hiding this comment

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

I have some minor copyediting nits that you can ignore if you wish.

LGTM.

///
/// Iterators are inherently stateful. Calling `next` "consumes" a value from
/// the Iterator that cannot be put back, so keep that in mind when sharing
/// iterators between consumers.
Copy link
Contributor

Choose a reason for hiding this comment

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

so keep that in mind when sharing iterators between consumers.

I'm more confused than helped by this warning.

Are you effectively saying that "an iterator is a mutable object; when shared, multiple users calling next will interfere"?

/// the Iterator that cannot be put back, so keep that in mind when sharing
/// iterators between consumers.
///
/// An iterater `i` can be iterated over using
Copy link
Contributor

Choose a reason for hiding this comment

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

passive voice. I prefer active voice, when possible. For instance: "A for loop expression consumes an iterator by using its body to consume each item produced by the iterator. The variable x (bound in the for body) ranges over these items. "

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

5 participants