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

compiler warning 3370 fixed #913

Closed
wants to merge 1 commit into from
Closed

Conversation

Fxplorer
Copy link
Contributor

When building, was receiving compiler warning 3370
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'.

Following the guidance, changes made to offending expressions.

All tests still showing green.

Before you go

Hi there! Thank you for your contribution to our project. To help maintain the quality of our codebase, our Continuous Integration (CI) system will automatically run several checks on your submission. To streamline the process, we kindly ask you to perform these checks locally before pushing your changes:

To execute all build scripts, please run:

dotnet fsi build.fsx

If you encounter any formatting issues, you can auto-correct them by running:

dotnet fantomas build.fsx src tests docs

Should any tests fail, please review and adjust your changes accordingly.

We appreciate your efforts to contribute and look forward to reviewing your pull request!

When building, was receiving `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'.`

Following the guidance, changes made to offending expressions.
Copy link
Collaborator

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

Thanks

@@ -1,5 +1,5 @@
{
"sdk": {
"version": "8.0.100"
"version": "8.0.202"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick: you should not bump the SDK here, it is unrelated to what you are trying to fix.
On the flip-side I'm not against having this, make it 8.0.200 and "rollForward": "latestMinor"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw that but I do not know how to force it down. The 8.0.202 sdk is what is installed on my dev environment and I had to change it to work. For future reference, should the intended changes be put into into a new (local) branch and then PR'd to make your (maintainers) job easier?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I saw that but I do not know how to force it down

I told you how to, add "rollForward": "latestMinor" and "version": "8.0.200" under "sdk.

Just because you installed .202 does not mean every other contributor needs that exact version. .200 and up should be fine.

@Fxplorer Fxplorer marked this pull request as draft March 29, 2024 16:33
@Fxplorer
Copy link
Contributor Author

replaced PR with #914

@Fxplorer Fxplorer closed this Mar 29, 2024
@nhirschey
Copy link
Collaborator

For future reference, should the intended changes be put into into a new (local) branch and then PR'd to make your (maintainers) job easier?

The intended changes should be new commits in this branch. Putting them in a different branch with a new PR makes things harder for maintainers.

@nojaf
Copy link
Collaborator

nojaf commented Mar 30, 2024

Hi @Fxplorer, I hope I didn't upset you or anything. The changes in #914 look fine and as @nhirschey mentioned, it would have been fine to have them here.

@Fxplorer
Copy link
Contributor Author

The real problem is that I do not yet understand how git works in situations of more than basic operations of saving and merging. The branching and cross pulls are still a subject that I need to learn. After some additional research, yes, I now understand that the original PR branch could be checked put again and updated.

At this this point, since I do not have the original branch and these both are closed, I have no idea how fix things.

@nojaf
Copy link
Collaborator

nojaf commented Mar 30, 2024

Because you have deleted the branch you will probably need to push your local branch (in case that one still exists) again to your remote.

Something like git push -u origin, and then you can raise a new PR.

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