Conversation
| protected void addStanzaSpecificAttributes(ToStringUtil.Builder builder) { | ||
| builder.addValue("type", type) | ||
| ; | ||
| builder.addValue("type", type); |
There was a problem hiding this comment.
That's an unrelated change in this commit. Also commit subject could be "[core] Fix typo in MessageBuilder"
There was a problem hiding this comment.
Such kind of change (obvious code style cleanup) is too small for a separate commit: too many of them will distract. I had to mention it in a commit message, but again it will be longer to describe than see.
There was a problem hiding this comment.
Such kind of change (obvious code style cleanup) is too small for a separate commit
It's not.
Flowdalic
left a comment
Flowdalic
left a comment
There was a problem hiding this comment.
I don't see the point in switching to String concatenation over StringBuilder. Yes, javac will probably transform String concatenation to StringBuilder and yes, one could argue it's slightly more readable. But am I not convinced that it's worth to change.
In general, please make sure that you don't conflate multiple changes within the same commit (or even, within the same PR). This makes it easier to review stuff, keeps the history clean and increases the chances for undisputed changes to land.
| controlLanguages.add(lang3); | ||
| controlLanguages.removeAll(languages); | ||
| assertTrue(controlLanguages.size() == 0); | ||
| assertTrue(controlLanguages.isEmpty()); |
| .addBody("es", "test") | ||
| .build(); | ||
| assertTrue(message.getBodies().size() == 1); | ||
| assertEquals(1, message.getBodies().size()); |
| var message = assertInstanceOf(Message.class, stanzas.get(0)); | ||
| // In this case, no xml:lang on the stream or the message and no explicit language argument provided to | ||
| // getBody(), it is anbiguous which body we should return. | ||
| // getBody(), it is ambiguous which body we should return. |
The javac will use concatenation with |
|
I am still skeptical that switching from |
stokito
force-pushed
the
message_builder
branch
from
9a8a97e to
ff709e1
Compare
|
I removed the commit with he StringBuilder |
There is still an unrelated change in the commit |
2 participants
Also cleanup MessageTest