Deprecate "network" and enable "networks" stats (remote Docker API 1.21) (branch master)#361
Deprecate "network" and enable "networks" stats (remote Docker API 1.21) (branch master)#361bonigarcia wants to merge 2 commits intodocker-java:masterfrom
Conversation
There was a problem hiding this comment.
Add java doc with @since to indicate when it appeared
There was a problem hiding this comment.
Object under question, doesn't it have some better structure?
There was a problem hiding this comment.
Sounds like over naming. Class itself about stats and naming vars with stats tail under question.
@marcuslinke imho better name it as is to be closer to docker API docs. Hopefully for this variable docker devs placed normal name.
There was a problem hiding this comment.
Variables are normally named like JSON fields in docker-java. But what about getter? getNetwork()/getNetworks() should return an dedicated object instead of a map, right?
There was a problem hiding this comment.
I see no specific structure and wrappers for it. Until it just a map iface:stats Map looks good. But i don't like <Object> how it expected for usage?
There was a problem hiding this comment.
Or do you want create the same analogue as PortBindings?
There was a problem hiding this comment.
There are API breaking changes already in master, so this will not work. We should really consider another branching model...
There was a problem hiding this comment.
@marcuslinke just create new branch based on previous to cc5b11a (hope it has no bad changes before) then @bonigarcia will PR to this branch and you will do 2.1.3 in parallel.
|
Closing because of duplicate PR (#362). Merged into master also. |
Superseed #360