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

Added tutorial on Lazy expressions to tutorial section of manual #2273

Merged
merged 8 commits into from Mar 5, 2019

Conversation

Projects
None yet
3 participants
@ulugbekna
Copy link
Contributor

commented Feb 27, 2019

Added a tutorial on Lazy expressions to the tutorial section of the manual. MPR#7547

Content based on:

  1. standard-library module Lazy
  2. Lazy patterns docs by @yallop
  3. Own experience and tests

I was also hesitant whether to make the link to module Lazy docs open in a new tab or the same. The former seems better UX.

Initially, I also wanted to add an example with a recursive fibonacci function call with a large value that would exemplify an expensive computation but decided not to for brevity.

@Octachron
Copy link
Contributor

left a comment

Thanks for your contribution! This looks nice, except for the lazy pattern part that might need to be a bit more gradual. (I have also some remarks on potential improvement earlier but you can ignore those ones).

Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
@ulugbekna

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2019

I have taken into account comments by @Octachron and fixed some parts of the tutorial. Thanks :)
Also did some changes in other places, where I felt they were due.

@gasche

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

(I trust @Octachron's judgment and review, and I think he will merge the PR once he declares himself satisfied.)

@Octachron
Copy link
Contributor

left a comment

I sill have few more remarks, but this is definitively converging towards a nice addition to the manual.

Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
Show resolved Hide resolved manual/manual/tutorials/coreexamples.etex Outdated
@Octachron

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

This looks nice, I will do a last round of review tomorrow (for spelling and typos mostly).

@gasche

gasche approved these changes Mar 4, 2019

Copy link
Member

left a comment

I made a minor nitpick, but with it the PR is good to go -- I'll merge when it is fixed.

We added "print_endline \"lazy_two evaluation\"" to see when the lazy
expression is being evaluated.

The value of "lazy_two" is "<lazy>", which means the expression is not evaluated

This comment has been minimized.

Copy link
@gasche

gasche Mar 4, 2019

Member

is displayed as "<lazy>" (this is not the actual value)

returns the plain value of the computation.

Now if we look at the
value of "lazy_two", we see that it is not "<lazy>" anymore but "lazy 2".

This comment has been minimized.

Copy link
@gasche

gasche Mar 4, 2019

Member

The value "lazy_two" is not displayed as "" anymore but "lazy 2".

The value of "lazy_two" is "<lazy>", which means the expression is not evaluated
yet, and the result is unknown.
The value of "lazy_two" is displayed as "<lazy>", which means the expression
has not been evaluated yet, and its actual value is unknown.

This comment has been minimized.

Copy link
@Octachron

Octachron Mar 4, 2019

Contributor

Final nitpick: maybe more final value than actual value?

This comment has been minimized.

Copy link
@ulugbekna

ulugbekna Mar 4, 2019

Author Contributor

Yes, that makes sense because "actual value" might also pertain to value displayed as .

@Octachron
Copy link
Contributor

left a comment

LGTM

@Octachron

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

@ulugbekna , before me (or @gasche) merge, would you mind adding a change entry in Changes (in a new ### Manual and documentation: subsection in the Working version section)? The entry format is described in see Contributing.md.

@ulugbekna

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

@Octachron, thanks for a heads-up! :-)

@Octachron

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

Could you add the GPR number to the change entry (i.e. MPR#7447, GPR#2273)?

@gasche , any objection to cherry-picking to 4.08?

@gasche

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

@Octachron none, especially if you do the work of merging and cherry-picking yourself. Thanks!

@Octachron Octachron merged commit 8b1fda5 into ocaml:trunk Mar 5, 2019

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Octachron added a commit that referenced this pull request Mar 5, 2019

Added tutorial on Lazy expressions to tutorial section of manual (#2273)
* Added tutorial on Lazy expressions to the tutorial of manual
* Modified tutorial on Lazy expressions to make it easier to understand
* Add small modifications in wording and a better lazy pattern matching example
* Add minor modification
* Change wording regarding <lazy> as a display of value
* Change a word
* Append Changes to mention the added tutorial
* Add GPR number
@Octachron

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

Merged and cherry-picked on 4.08 as d11878c.
@ulugbekna , thanks a lot for your contribution!

@ulugbekna

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

@Octachron and @gasche, thank you a lot for guiding through my first contribution. I appreciate your help and reviews :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.