Skip to content

Conversation

@JYCabello
Copy link
Contributor

Just to have some early feedback, or merge if it's OK.

@dnfadmin
Copy link

dnfadmin commented Sep 9, 2021

CLA assistant check
All CLA requirements met.

@JYCabello
Copy link
Contributor Author

I did this for this one #12124, I took from the top instead of the bottom. My bad

Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

Great work! Some initial feedback

///
/// <example>
/// <code lang="fsharp">
/// [(fun () -> Seq.ofList [1; 2; 3]) |> Seq.delay // evaluates to seq [1; 2; 3], calling the gnerator every time.
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra leading [

You wouldn't use pipelining with Seq.delay

Copy link
Contributor Author

@JYCabello JYCabello Sep 15, 2021

Choose a reason for hiding this comment

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

I'll change, but would like to know why, if you don't mind. (as in: I want to learn more)

@dsyme
Copy link
Contributor

dsyme commented Sep 15, 2021

@JYCabello I made one update - we can merge this when it's green

On re-read the compareWith examples could perhaps be better - they currently use the default comparison semantics for integers - we should change to examples which say compare the number x % 3 or something

@JYCabello
Copy link
Contributor Author

@JYCabello I made one update - we can merge this when it's green

On re-read the compareWith examples could perhaps be better - they currently use the default comparison semantics for integers - we should change to examples which say compare the number x % 3 or something

Will do on the next PR.

@dsyme dsyme merged commit 37b0a4d into dotnet:main Sep 15, 2021
@dsyme
Copy link
Contributor

dsyme commented Sep 15, 2021

Great work @JYCabello, thank you so much for this contribution.

@JYCabello JYCabello deleted the docs/seq_samples branch September 15, 2021 12:57
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.

3 participants