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

C# 7.x: Adding rules for improved overload candidates #263

Merged
merged 5 commits into from
Nov 2, 2022
Merged

C# 7.x: Adding rules for improved overload candidates #263

merged 5 commits into from
Nov 2, 2022

Conversation

RexJaeschke
Copy link
Contributor

@RexJaeschke RexJaeschke commented Apr 18, 2021 •
edited by BillWagner
Loading

Fixes #188
Fixes #499

@RexJaeschke RexJaeschke added this to the C# 7.x milestone Apr 18, 2021
@RexJaeschke RexJaeschke added the Review: pending Proposal is available for review label Sep 16, 2021
@RexJaeschke RexJaeschke mentioned this pull request Jan 3, 2022
Open
@BillWagner BillWagner marked this pull request as draft February 3, 2022 15:16
@BillWagner
Copy link
Member

converted to draft: C# 7 feature

@BillWagner BillWagner force-pushed the Rex-v7-improved-overload-candidates branch from 8460d82 to 3e14536 Compare April 3, 2022 19:34
@gafter gafter self-assigned this Apr 13, 2022
gafter
gafter reviewed May 11, 2022
View reviewed changes
standard/expressions.md Outdated Show resolved Hide resolved
gafter
gafter reviewed May 11, 2022
View reviewed changes
Copy link
Member

@gafter gafter left a comment

Choose a reason for hiding this comment

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

Looks good except as noted.

@gafter gafter added Review: complete at least one person has reviewed this and removed Review: pending Proposal is available for review labels May 11, 2022
@gafter gafter assigned gafter and unassigned gafter May 11, 2022
@gafter gafter mentioned this pull request May 13, 2022
Open
5 tasks
@jskeet jskeet marked this pull request as ready for review June 15, 2022 12:41
@jskeet
Copy link
Contributor

jskeet commented Jun 15, 2022

Opened up for broader review given Neal's label.

@BillWagner
Copy link
Member

This PR will address #499

BillWagner
BillWagner approved these changes Jun 15, 2022
View reviewed changes
Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

A had two suggestions. One is a possible typo, the second is suggested wording for Neal's comment.

standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
@BillWagner
Copy link
Member

BillWagner commented Jun 15, 2022 •
edited
Loading

We can address it in a second PR, but the text changes here are related to #552

The sentences noted in that issue in 10.8 are resolved by these changes. I believe the second two quoted sentences can be removed, and a link added to this section.

@BillWagner BillWagner mentioned this pull request Jun 15, 2022
Closed
jskeet
jskeet reviewed Jul 13, 2022
View reviewed changes
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
Nigel-Ecma
Nigel-Ecma reviewed Jul 13, 2022
View reviewed changes
standard/expressions.md Outdated Show resolved Hide resolved
Nigel-Ecma
Nigel-Ecma reviewed Jul 13, 2022
View reviewed changes
standard/expressions.md Outdated Show resolved Hide resolved
@jskeet jskeet assigned jskeet and unassigned gafter Jul 13, 2022
@jskeet
Copy link
Contributor

jskeet commented Jul 13, 2022

Jon to apply selected tweaks and send out for review again.

@jskeet jskeet added Review: work pending and removed Review: complete at least one person has reviewed this labels Jul 13, 2022
jskeet
jskeet reviewed Aug 23, 2022
View reviewed changes
Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

I've made some progress and pushed a new commit, but I think we need more clarity around what's meant by an "instance receiver". (I'm sure we all know what it's intended to mean, but I don't think we define it clearly enough.)

standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
Nigel-Ecma
Nigel-Ecma reviewed Aug 26, 2022
View reviewed changes
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
@BillWagner BillWagner force-pushed the Rex-v7-improved-overload-candidates branch from b266c7f to 4d97e68 Compare October 2, 2022 22:16
jskeet
jskeet reviewed Oct 5, 2022
View reviewed changes
Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Okay, a bit more progress, but we'll definitely want to discuss this in more detail.

standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
jskeet
jskeet reviewed Oct 5, 2022
View reviewed changes
standard/expressions.md Outdated Show resolved Hide resolved
@jskeet jskeet mentioned this pull request Oct 5, 2022
Closed
@jskeet
Copy link
Contributor

jskeet commented Oct 26, 2022

Okay, I've had another pass at this, to remove the undefined term "receiver". PTAL before the next meeting (where we'll discuss it).

@jskeet jskeet added meeting: discuss This issue should be discussed at the next TC49-TG2 meeting Review: pending Proposal is available for review and removed Review: work pending labels Oct 26, 2022
@jskeet jskeet merged commit 0ca94a5 into draft-v7 Nov 2, 2022
@jskeet jskeet deleted the Rex-v7-improved-overload-candidates branch November 2, 2022 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jskeet jskeet jskeet left review comments

@BillWagner BillWagner BillWagner approved these changes

@gafter gafter gafter left review comments

@Nigel-Ecma Nigel-Ecma Nigel-Ecma left review comments

@ericlippert ericlippert Awaiting requested review from ericlippert

@MadsTorgersen MadsTorgersen Awaiting requested review from MadsTorgersen

Assignees

@jskeet jskeet

Labels
meeting: discuss This issue should be discussed at the next TC49-TG2 meeting Review: pending Proposal is available for review
Projects
None yet
Milestone
C# 7.x
5 participants
@RexJaeschke @BillWagner @jskeet @gafter @Nigel-Ecma