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

Fake.Core.Process: stop using old ref-cell operators #2674

Merged

Conversation

knocte
Copy link
Contributor

@knocte knocte commented May 6, 2022

The new version of the F# compiler now flags these occurrences as deprecated.

The messages are of two kinds:

  1. info FS3370: The use of ':=' from the F# library is deprecated.
    See https://aka.ms/fsharp-refcell-ops.
    For example, please change 'cell := expr' to 'cell.Value <- expr'.

  2. info FS3370: The use of '!' from the F# library is deprecated.
    See https://aka.ms/fsharp-refcell-ops.
    For example, please change '!cell' to 'cell.Value'.

@knocte knocte force-pushed the avoidOldRefCellOperators branch 2 times, most recently from d21651c to 88f8ed4 Compare May 9, 2022 14:26
@knocte
Copy link
Contributor Author

knocte commented May 9, 2022

Oops, CI was failing because I made a typo (forgot a <- operator) in one line. Fixed. CI should be green now.

@yazeedobaid
Copy link
Collaborator

Hi thanks for the PR
Could you please rebase your branch from release/next? Now that the build has been fixed.

The new version of the F# compiler now flags these occurrences as
deprecated.

The messages are of two kinds:

1) info FS3370: The use of ':=' from the F# library is deprecated.
See https://aka.ms/fsharp-refcell-ops.
For example, please change 'cell := expr' to 'cell.Value <- expr'.

2) info FS3370: The use of '!' from the F# library is deprecated.
See https://aka.ms/fsharp-refcell-ops.
For example, please change '!cell' to 'cell.Value'.
@knocte
Copy link
Contributor Author

knocte commented Jul 16, 2022

Could you please rebase your branch from release/next?

Done.

@yazeedobaid
Copy link
Collaborator

Can you please check if other parts of the FAKE repo have usage for these operators and replace them the same way?
If it is a lot of work, I'm ok with merging this PR, and then we can open an issue to track other parts and merge them in small chunks. Or in this PR. Whatever works best for you.

@yazeedobaid yazeedobaid added the in-review Convey that a PR is in-review status label Jul 21, 2022
@knocte
Copy link
Contributor Author

knocte commented Jul 26, 2022

% grep ":=" -ri . | grep -v legacy | grep -v Binary
./src/app/Fake.Runtime/YaafFSharpScripting.fs:          __hook := obj
./src/app/Fake.Runtime/YaafFSharpScripting.fs:                  isDisposed := true }
./src/app/Fake.Runtime/YaafFSharpScripting.fs:                  isDisposed := true }
./src/app/Fake.Runtime/YaafFSharpScripting.fs:              properEndLine := false
./src/app/Fake.Runtime/YaafFSharpScripting.fs:              properEndLine := true
./src/app/Fake.Runtime/ScriptRunner.fs:    //__hook := execContext
./src/app/Fake.Runtime/LegacyApiHelper.fs:let ofRef r = { Set = (fun v -> r := v); Get = (fun () -> !r) }
./src/app/Fake.DotNet.FxCop/FxCop.fs:      Item "/ruleset:=%s" param.CustomRuleset
./src/app/Fake.IO.FileSystem/Path.fs:            resultPath := !resultPath + directorySeparator + ".."
./src/app/Fake.IO.FileSystem/Path.fs:            baseLocation := Path.GetDirectoryName !baseLocation
./src/app/Fake.IO.FileSystem/Path.fs:                baseLocation := (!baseLocation).Substring(0, (!baseLocation).Length - 1)
./src/app/Fake.IO.FileSystem/Path.fs:        := (!resultPath + targetLocation.Substring((!baseLocation).Length)) 
./src/app/Fake.IO.FileSystem/ChangeWatcher.fs:                    unNotifiedChanges := []
./src/app/Fake.IO.FileSystem/ChangeWatcher.fs:                        runningHandlers := true
./src/app/Fake.IO.FileSystem/ChangeWatcher.fs:                        runningHandlers := false )
./src/app/Fake.IO.FileSystem/ChangeWatcher.fs:                  unNotifiedChanges := fileChange :: !unNotifiedChanges
./src/app/Fake.IO.FileSystem/Shell.fs:                i := !i + 1
./src/app/Fake.JavaScript.Npm/Npm.fs:            if exitCode <> 0 then result := Some(sprintf "exit code: %d" exitCode)
./src/app/Fake.JavaScript.Npm/Npm.fs:            if not (isNull exn.InnerException) then message := !message + Environment.NewLine + exn.InnerException.Message
./src/app/Fake.JavaScript.Npm/Npm.fs:            result := Some(!message)
./src/app/Fake.IO.Zip/Zip.fs:        length := !length - (int64 count)
./src/app/Fake.Core.CommandLineParsing/docopt.fs/Docopt/OptionsParser.fs:        lastOpt := Some opt
./src/app/Fake.Core.Process/InternalStreams.fs:                    first := false
./src/app/Fake.Core.Process/InternalStreams.fs:                        closed := true } |> Async.Start
./src/app/Fake.Core.Process/InternalStreams.fs:        //                current := next
./src/app/Fake.Core.Process/InternalStreams.fs:                            closeRead :=
./src/app/Fake.Core.Process/InternalStreams.fs:                                    streamFinished := true
./src/app/Fake.Core.Process/InternalStreams.fs:                    closeRead := true
./src/app/Fake.Core.Process/Process.fs:                              traced := true
./src/app/Fake.Core.Process/Process.fs:            messages := { IsError = isError
./src/app/Fake.Core.Process/GuardedAwaitObservable.fs:        let setRemover r = lock removeLock (fun () -> removeObj := Some r)
./src/app/Fake.Core.Process/GuardedAwaitObservable.fs:                    removeObj := None
./src/test/Fake.Core.UnitTests/Fake.DotNet.FxCop.fs:                    wrap "/ruleset:=" p.CustomRuleset

Seems like a lot of work to me, I'd prefer if other contributor finishes it.

@yazeedobaid
Copy link
Collaborator

Thanks for the PR, other places that use old ref-cell operators are replaced in v6 branch.

@yazeedobaid yazeedobaid merged commit 817396e into fsprojects:release/next Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-review Convey that a PR is in-review status
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants