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: private protected access modifier #215

Merged
merged 8 commits into from
Sep 7, 2022
Merged

C# 7.x: private protected access modifier #215

merged 8 commits into from
Sep 7, 2022

Conversation

RexJaeschke
Copy link
Contributor

No description provided.

@RexJaeschke RexJaeschke added this to the C# 7.x milestone Jan 26, 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 14:53
@BillWagner
Copy link
Member

C# 7 feature. Converting to a draft.

@gafter gafter self-assigned this Apr 13, 2022
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.

I noticed some missing rules, but other than as noted looks good.

standard/classes.md Outdated Show resolved Hide resolved
standard/classes.md Show resolved Hide resolved
standard/classes.md Show resolved Hide resolved
standard/classes.md Show resolved Hide resolved
@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
gafter
gafter reviewed Jun 8, 2022
View reviewed changes
standard/classes.md Outdated Show resolved Hide resolved
@RexJaeschke RexJaeschke mentioned this pull request Jun 10, 2022
Open
@jskeet
Copy link
Contributor

jskeet commented Jun 15, 2022

Opening up for broader review given Neal's label.

@jskeet jskeet marked this pull request as ready for review June 15, 2022 12:42
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.

I'll approve, with the recommendation that we add #594 to the C# 7 milestone.

jskeet
jskeet reviewed Jul 13, 2022
View reviewed changes
standard/classes.md Outdated Show resolved Hide resolved
standard/classes.md Outdated Show resolved Hide resolved
standard/classes.md Show resolved Hide resolved
standard/classes.md Outdated Show resolved Hide resolved
standard/classes.md Show resolved Hide resolved
standard/classes.md Show resolved Hide resolved
KalleOlaviNiemitalo
KalleOlaviNiemitalo suggested changes Jul 13, 2022
View reviewed changes
standard/basic-concepts.md Outdated Show resolved Hide resolved
Nigel-Ecma
Nigel-Ecma requested changes Jul 13, 2022
View reviewed changes
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

Just a couple of fixes, otherwise LGTM

standard/basic-concepts.md Outdated Show resolved Hide resolved
standard/basic-concepts.md Outdated Show resolved Hide resolved
standard/classes.md Outdated Show resolved Hide resolved
@jskeet
Copy link
Contributor

jskeet commented Jul 13, 2022

Jon to modify text slightly as indicated.

@jskeet jskeet assigned jskeet and unassigned gafter Jul 13, 2022
@jskeet jskeet added Review: work pending and removed Review: complete at least one person has reviewed this labels Jul 13, 2022
@jskeet jskeet mentioned this pull request Jul 14, 2022
Open
jskeet
jskeet reviewed Jul 26, 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 added a new commit with all the changes noted. PTAL.

standard/basic-concepts.md Outdated Show resolved Hide resolved
standard/basic-concepts.md Outdated Show resolved Hide resolved
standard/classes.md Outdated Show resolved Hide resolved
standard/classes.md Outdated Show resolved Hide resolved
gafter
gafter requested changes Sep 6, 2022
View reviewed changes
standard/unsafe-code.md Outdated Show resolved Hide resolved
Nigel-Ecma
Nigel-Ecma requested changes Sep 7, 2022
View reviewed changes
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

Added wording suggestion to @gafter's issue on unsafe-code#887. Rest LGTM

Nigel-Ecma
Nigel-Ecma approved these changes Sep 7, 2022
View reviewed changes
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

LGTM. @gafter?

gafter
gafter approved these changes Sep 7, 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.

🐸

BillWagner
BillWagner approved these changes Sep 7, 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.

After commit 8, this LGTM.

@jskeet jskeet merged commit 6c3ddde into draft-v7 Sep 7, 2022
@jskeet jskeet deleted the Rex-v7-private-protected branch September 7, 2022 20:30
@BillWagner BillWagner mentioned this pull request Sep 8, 2022
Merged
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

@KalleOlaviNiemitalo KalleOlaviNiemitalo KalleOlaviNiemitalo requested changes

@BillWagner BillWagner BillWagner approved these changes

@gafter gafter gafter approved these changes

@Nigel-Ecma Nigel-Ecma Nigel-Ecma approved these changes

@ericlippert ericlippert ericlippert approved these changes

@MadsTorgersen MadsTorgersen Awaiting requested review from MadsTorgersen

Assignees

@jskeet jskeet

Labels
None yet
Projects
None yet
Milestone
C# 7.x
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@RexJaeschke @BillWagner @jskeet @gafter @Nigel-Ecma @ericlippert @KalleOlaviNiemitalo