-
-
Notifications
You must be signed in to change notification settings - Fork 606
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
Add getVisibility trait as an alias to getProtection #12089
Conversation
The name is more accurate and can be recommended in the documentation in the future.
Thanks for your pull request, @Geod24! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#12089" |
Does it work cross module now? There was talk before about changing it to do (aggr, "name") as a string instead of aggr.name because of that. I don't remember if that was already changed to work regardless but if not this would be a good chance to adjust that too. |
True. Another thing that comes to mind is |
Behavior changes must be in a separate PR. |
On Fri, Jan 01, 2021 at 06:23:09PM -0800, Walter Bright wrote:
Behavior changes must be in a separate PR.
If you're introducing a new name, that's a good opportunity to fix
design flaws without any breaking changes. Keep the old name the same
and make the new name fix things.
If you introduce the new name first, then any fixing of its design
becomes a breaking change. So we should make sure the design is OK
before rushing ahead.
But that said it seems to be ok now anyway... traits(getMember)
bypasses the visibility check so I think the current behavior is OK...
|
The name is more accurate and can be recommended in the documentation in the future.