[Softlayer] Implements setting and retrieving the notes property #756
Conversation
neykov
force-pushed
the
sl-notes
branch
5 times, most recently
from
6c9b19b
to
8a9ae16
Compare
| private final Json json; | ||
|
|
||
| @Inject | ||
| public NotesToJson(Json json) { |
There was a problem hiding this comment.
Remove the modifier to make the constructor package private, and remove the null check.
|
Addressed part of the comments, waiting for input on the rest. |
|
I've just commented on the remaining bits. Also take #763 into account when rebasing, just in case Andrea merges that PR first, as it seems to overlap with some of your changes. |
|
Addressed comments. To set notes in SL the user should use |
|
|
||
| @Inject | ||
| NotesToJson(Json json) { | ||
| this.json = checkNotNull(json, "json"); |
There was a problem hiding this comment.
Once the constructor is only visible to the injector, the null check is redundant (the injector already checks nulls). Remove it.
|
Just a couple minors and one comment left. Apart from that lgtm. Thanks @neykov! Mind squashing the commits into a single one when addressing the changes so I can cleanly merge it? |
|
Addressed comments and squashed into a single commit. |
|
Almost there! :) It looks that there is a test failing now. Mind fixing it? |
|
Looking good now :) |
No description provided.