Skip to content

Samples for seq module#12133

Merged
dsyme merged 22 commits into
dotnet:mainfrom
JYCabello:docs/seq_samples
Sep 15, 2021
Merged

Samples for seq module#12133
dsyme merged 22 commits into
dotnet:mainfrom
JYCabello:docs/seq_samples

Conversation

@JYCabello

Copy link
Copy Markdown
Contributor

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

@dnfadmin

dnfadmin commented Sep 9, 2021

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@JYCabello

Copy link
Copy Markdown
Contributor Author

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

@dsyme dsyme left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
///
/// <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
Copy Markdown
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

@JYCabello JYCabello Sep 15, 2021

Copy link
Copy Markdown
Contributor Author

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)

Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
Comment thread src/fsharp/FSharp.Core/seq.fsi Outdated
@dsyme

dsyme commented Sep 15, 2021

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
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

dsyme commented Sep 15, 2021

Copy link
Copy Markdown
Contributor

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