Skip to content

Reserve an error message for "obj was inferred" - #14642

Merged
T-Gro merged 11 commits into
dotnet:mainfrom
Smaug123:obj-inference-error-message
Jan 31, 2023
Merged

Reserve an error message for "obj was inferred"#14642
T-Gro merged 11 commits into
dotnet:mainfrom
Smaug123:obj-inference-error-message

Conversation

@Smaug123

@Smaug123Smaug123 commented Jan 22, 2023

Copy link
Copy Markdown
Contributor

This is boilerplate around fsharp/fslang-suggestions#696 .

The actual implementation is #13298, but that has rotted several times. I'd like to get this boilerplate out of the way so that it stops rotting.

@Smaug123
Smaug123 requested a review from a team as a code ownerJanuary 22, 2023 13:56
psfinaki
psfinaki reviewed Jan 23, 2023
View reviewed changes
Comment threadsrc/Compiler/FSComp.txt Outdated
Smaug123
Smaug123 commented Jan 26, 2023
View reviewed changes
Comment threadtests/FSharp.Test.Utilities/Compiler.fs
@Smaug123

Smaug123 commented Jan 26, 2023

Copy link
Copy Markdown
ContributorAuthor

Now "A type has been implicitly inferred as obj, which may be unintended" plus the "consider disabling".

@Smaug123

Smaug123 commented Jan 26, 2023

Copy link
Copy Markdown
ContributorAuthor

I would like to get this in at some point in the near future, because this is the part of the change that rots the fastest.

@psfinaki

psfinaki commented Jan 27, 2023

Copy link
Copy Markdown
Contributor

I am fine with the wording now. @dotnet/fsharp-team-msft ping

abelbraaksma
abelbraaksma suggested changes Jan 30, 2023
View reviewed changes

@abelbraaksmaabelbraaksma 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.

Small nitpick, see below (I didn't repeat the comment for all translation files, these are generated automatically anyway). Otherwise good.

Comment threadsrc/Compiler/FSComp.txt Outdated
Comment threadsrc/Compiler/xlf/FSComp.txt.cs.xlf
abelbraaksma
abelbraaksma approved these changes Jan 31, 2023
View reviewed changes

@abelbraaksmaabelbraaksma 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.

Looking good! I think this can be merged @vzarytovskii, @dsyme, @KevinRansom

@T-Gro
T-Gro merged commit 8f26536 into dotnet:mainJan 31, 2023
@Smaug123
Smaug123 deleted the obj-inference-error-message branch January 31, 2023 19:29
@Smaug123Smaug123 mentioned this pull request Jan 31, 2023
Closed
5 tasks
@allisonchouallisonchou mentioned this pull request Feb 17, 2023
Closed
@Smaug123Smaug123 mentioned this pull request Apr 12, 2023
Merged
7 tasks
@KevinRansomKevinRansom mentioned this pull request Jun 16, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@T-GroT-GroT-Gro approved these changes
+2 more reviewers
@psfinakipsfinakipsfinaki approved these changes
@abelbraaksmaabelbraaksmaabelbraaksma approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release/17.6

Projects

F# Compiler and Tooling
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Smaug123@psfinaki@abelbraaksma@T-Gro@vzarytovskii