Fix bugs in programmatic cluster config. Add unit test.#368
Fix bugs in programmatic cluster config. Add unit test.#368k8s-ci-robot merged 1 commit intokubernetes-client:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| } | ||
|
|
||
| public addUser(user: User) { | ||
| if (!this.users) { |
There was a problem hiding this comment.
Would it not be better to put this in the constructor? I think it would be more logical and easier to follow too.
There was a problem hiding this comment.
Not a blocker just curious before I add my lgtm (and it merges)
There was a problem hiding this comment.
The fields involved are public, so it's always possible that someone will overwrite them with something undefined or null, so the check here is safer.
6665458 to
cf6c628
Compare
|
Comment addressed, please re-check. Thanks! |
|
/lgtm |
Fixes #365