More info classes to implement Writeable rather than Streamable #20288
Conversation
javanna
changed the title
| } | ||
|
|
||
| public PluginsAndModules(StreamInput in) throws IOException { | ||
| int pluginsSize = in.readInt(); |
There was a problem hiding this comment.
Could this be plugins = in.readList(PluginInfo::new)?
|
Changes LGTM. I left a bunch of minor stuff but none of them are actual objections. I didn't carefully match up all the changes you did to make sure you added tests for them. Do you want me to double check that? |
javanna
force-pushed
the
enhancement/more_writeable
branch
from
980edbc
to
5451c31
Compare
please do. I didn't add many specific tests, rather beefed up |
| @@ -302,6 +302,14 @@ public static int randomInt() { | |||
| return random().nextInt(); | |||
| } | |||
|
|
|||
| public static long randomPositiveLong() { | |||
| long positiveLong = randomLong(); | |||
There was a problem hiding this comment.
ooh! another wonderful reason to use do {...} while syntax! The rarest of syntax!
There was a problem hiding this comment.
Also maybe change the variable name. The value might not be positive here.
|
OK! I rechecked. LGTM. |
javanna
force-pushed
the
enhancement/more_writeable
branch
from
941556a
to
4bac777
Compare
With this we can make ByteSizeValue immutable for real.
This allows to make all instance members final. Also added serialization tests and sorted out inizialization that was scattered in two places.
… existing NodeInfoStreamingTests
javanna
force-pushed
the
enhancement/more_writeable
branch
from
79aeecf
to
51620f7
Compare
While working on #20255 I noticed that we could easily migrate some classes that belong to our info api from
StreamabletoWriteable. I did it for some of them in this PR. That allows to have final members and more concise / better looking code. Also added tests where needed.