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.

Sign up for GitHub

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

Jump to bottom

Add support for ValueTask #523

Merged
merged 32 commits into from
Nov 28, 2022
Merged

Add support for ValueTask #523

merged 32 commits into from
Nov 28, 2022

Conversation

rodriguestiago0
Copy link
Contributor

No description provided.

@rodriguestiago0 rodriguestiago0 marked this pull request as draft November 23, 2022 21:24
@gusty gusty changed the title First try for value task Add support for ValueTask Nov 24, 2022
@rodriguestiago0 rodriguestiago0 marked this pull request as ready for review November 24, 2022 13:57
gusty
gusty reviewed Nov 24, 2022
View reviewed changes
Copy link
Member

@gusty gusty left a comment

Choose a reason for hiding this comment

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

Look good so far.
Please update the docs and we're done !

src/FSharpPlus/FSharpPlus.fsproj Outdated Show resolved Hide resolved
@wallymathieu
Copy link
Member

wallymathieu commented Nov 26, 2022 •
edited
Loading

I've split the net7 upgrade into #525 (to see if net 7 causes the fable issue)

abelbraaksma
abelbraaksma reviewed Nov 26, 2022
View reviewed changes
Copy link
Member

@abelbraaksma abelbraaksma left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There’s a line has a check warning. Is that new with net7?

Check warning on line 43 in src/FSharpPlus/Control/Monoid.fs

 GitHub Actions
/ testFable3SubsetOnCore

This upcast is unnecessary - the types are identical

Also:

Possible incorrect indentation: this token is offside of context started at position (234:1).

And it appears that Fable3 tests don’t finish. @gusty is that common?

Fable / testfable3 (pull_request) Cancelled after 360m

@wallymathieu
Copy link
Member

The issue with fable tests not completing seems to be related to having net7 installed. It works fine if you only have net6.

@gusty
Copy link
Member

gusty commented Nov 26, 2022

And it appears that Fable3 tests don’t finish. @gusty is that common?

No @abelbraaksma is not, I will report this to the F# team, it might be the mysterious bug they are hunting for, where the compilation never stops.

In the meantime, the workaround is here: #525

@gusty gusty merged commit 3c05e4b into fsprojects:master Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gusty gusty gusty left review comments

@abelbraaksma abelbraaksma abelbraaksma left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rodriguestiago0 @wallymathieu @gusty @abelbraaksma