Clarify PermissionGroup.owner is @Nonnull #2805

Merged
merged 1 commit into from Mar 17, 2017

Conversation

3 participants
@batmat
Member

batmat commented Mar 14, 2017

I was reading the Permission class and realized that:

Description

Details: Mark Permission.owner as @nonnull

Changelog entries

Proposed changelog entries:

  • Mark PermissionGroup.owner @nonnull (note: can not change current behaviour as this is only a compiler/IDE hint)

Submitter checklist

  • JIRA issue is well described
  • Link to JIRA ticket in description, if appropriate
  • Appropriate autotests or explanation to why this change has no tests
  • For new API and extension points: Link to the reference implementation in open-source (or example in Javadoc)

Desired reviewers

@reviewbybees

@reviewbybees

This comment has been minimized.

Show comment
Hide comment
@reviewbybees

reviewbybees Mar 14, 2017

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@@ -53,7 +55,7 @@
* @param title sets {@link #title}
* @throws IllegalStateException if this group was already registered
*/
- public PermissionGroup(Class owner, Localizable title) throws IllegalStateException {
+ public PermissionGroup(@Nonnull Class owner, Localizable title) throws IllegalStateException {

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Mar 14, 2017

Member

Maybe it worth marking the class field as Nonnull as well

@oleg-nenashev

oleg-nenashev Mar 14, 2017

Member

Maybe it worth marking the class field as Nonnull as well

This comment has been minimized.

@batmat

batmat Mar 15, 2017

Member

Yeah, right indeed. Just did it. Thanks

@batmat

batmat Mar 15, 2017

Member

Yeah, right indeed. Just did it. Thanks

Clarify PermissionGroup.owner is @nonnull
Permission.owner is already marked @nonnull, and
is built using PermissionGroup.owner.
@oleg-nenashev

🐝

@batmat

This comment has been minimized.

Show comment
Hide comment
Member

batmat commented Mar 17, 2017

@batmat batmat merged commit 0717285 into jenkinsci:master Mar 17, 2017

0 of 2 checks passed

Jenkins Jenkins is validating pull request ...
Details
continuous-integration/jenkins/pr-head This commit is being built
Details

@batmat batmat deleted the batmat:clarify_PermissionGroup.owner_is_Nonnull branch Mar 17, 2017

@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Mar 17, 2017

Member

Thanks Oleg!

Member

batmat commented Mar 17, 2017

Thanks Oleg!

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Mar 17, 2017

Member

Thanks Baptiste! :)

Member

oleg-nenashev commented Mar 17, 2017

Thanks Baptiste! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment