Exposed details of the CommandPermission#523
Conversation
Made the optional string permission in the CommandPermission public
willkroboth
left a comment
There was a problem hiding this comment.
Definitely works as expected.
Would it be useful to make these other methods in CommandPermission public?
I'm not sure if it is necessary to mention this method in the docs: https://commandapi.jorel.dev/9.3.0/permissions.html. Maybe at least add javadocs to the method now that it's public.
You could also add this change to the Changelog in the README:
|
Probably don't, because the permissionNode can be null, and nullables should not be exposed according to the contribution guide and Skepter on Discord. |
Makes sense. I suppose you could also change the method signature of The
Yeah, you wouldn't change the old docs since the method wouldn't be public in 9.3.0. You'd change the source file here: https://github.com/JorelAli/CommandAPI/blob/46d1fa14990f9d3b6b9611e08f4e953f886291c9/docssrc/src/permissions.md. Then once the 9.4.0 docs are generated from the source files the change will be visible. But also yeah, I'm not sure if it's a big enough deal to mention?
I meant a Javadoc on the |
I wouldn't change the signature of getPermissionNode because, as you mentioned, they could just check with the static instances. And for those who, for any reason, use reflection to access it, it remains accessible.
Yea, makes sense.
It doesn't really fit in the docs, so I would just not mention it there.
Makes sense. I thought you meant we should mention the change somewhere in the Javadocs. |
Added javadoc
| /** | ||
| * Returns if the permission is negated | ||
| * | ||
| * @return the permissions negation state |
There was a problem hiding this comment.
"permissions" is missing an apostrophe.
Made the optional string permission and negation state in the CommandPermission public