-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Categories in "Accessibility" are incorrect #6207
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
Conversation
-The brief provided for "private protected" should be the brief for "protected internal" instead. -As per my knowledge in c#, "private protected" access modifier does not exist. If it does what does this exactly do?
| * `private` | ||
| - Access limited to this class | ||
| * `private protected` | ||
| - Access limited to the containing class or classes derived from the containing type within the same assembly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? private protected access modifier does exist, it was invented in C# 7.2
See this doc for the reference.
The only change I would do here is add such reference for each access modifier directly in this Accessibility section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be "nice to have", but I'll merge the PR if the text in the protected internal is fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@d-dizhevsky Thanks for my knowledge update. It helped.
@Bill Wagner Added private protected back in latest commit, with its original brief.
| - Access limited to the current assembly (.exe, .dll, etc.) | ||
| * `protected internal` | ||
| - Access limited to the containing class or classes derived from the containing class | ||
| - Access limited to the containing class or classes derived from the containing type within the same assembly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both the old version and the new version are wrong.
What it should say is that protected internal means that access is limited to:
- the containing class
- classes derived from the containing class
- types within the same assembly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The analysis by @svick above is correct. I would write this as: "Access limited to the containing class, classes derived from the containing class, or classes within the same assembly."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BillWagner Updated as per your review comment. You can see the required change in latest commit.
BillWagner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for creating this and fixing it @aditimantri2196
We apprecia6te it. There's one change I"d like you to make before I
. If you updated line 61, and sync that branch, the PR will be updated. I'll approve the changes and
.
Just @ - mention me if you have any questions. Thanks again.
| - Access limited to the current assembly (.exe, .dll, etc.) | ||
| * `protected internal` | ||
| - Access limited to the containing class or classes derived from the containing class | ||
| - Access limited to the containing class or classes derived from the containing type within the same assembly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The analysis by @svick above is correct. I would write this as: "Access limited to the containing class, classes derived from the containing class, or classes within the same assembly."
| * `private` | ||
| - Access limited to this class | ||
| * `private protected` | ||
| - Access limited to the containing class or classes derived from the containing type within the same assembly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be "nice to have", but I'll merge the PR if the text in the protected internal is fixed.
-Updated brief for protected internal as per the review comment -Added private protected back with some modification as per the reference doc.
Added back the private protected with its original brief.
|
@BillWagner Made the requested change. |
Updated the number of categories of accessiblility from five to six.
|
Thanks for making the updates @aditimantri2196 I've reviewed the latest version, I'll You should see the changes on the live site in the next few days, on our regular publishing cycle. Thanks again. |
Summary
-The brief provided for "private protected" should be the brief for "protected internal".
-Removed "private protected" from the category.
Fixes #6206