Conversation
|
@JemDay LGTM. This is a neat contribution, thanks! |
|
Thanks @Alfusainey - I do need to clean up the documentation side of it but if there's anything else that appears glaringly wrong pls let me know. @pierDipi - if you could start taking a peek at this I'd appreciate it. |
| /** | ||
| * Get the default content type to assume for MQTT messages. | ||
| * @return A Content-Type | ||
| */ | ||
| public static final String getDefaultContentType() { | ||
| return DEFAULT_FORMAT; | ||
| } |
There was a problem hiding this comment.
what's rationale for using a public static method vs referencing the field directly and make it public?
| // No-Op | ||
| return null; |
There was a problem hiding this comment.
Should we throw an exception in this case mostly for "internal safety"?
There was a problem hiding this comment.
I think I need to do some general clean-up ..
As an aside do we know why the definition of a 'MessageWriter' interface is so complicated, I'm not sure why it needs to extend other writers and factories.
pierDipi
left a comment
pierDipi
left a comment
There was a problem hiding this comment.
On multiple files the copyright header is missing:
sdk-java/api/src/main/java/io/cloudevents/lang/Nullable.java
Lines 1 to 16 in 4c81f3e
bindings/mqtt/core/src/main/java/io/cloudevents/mqtt/core/BaseMqttBinaryMessageReader.java
Show resolved
Hide resolved
bindings/mqtt/core/src/main/java/io/cloudevents/mqtt/core/MqttUtils.java
Outdated
Show resolved
Hide resolved
|
btw, I'm still going through it |
|
thx ... I'll try and take a look at this next week now that the proto changes are in ... |
Signed-off-by: Jem Day <Jem.Day@cliffhanger.com>
3 participants
For comment at present - DO NOT MERGE.
Contains MQTT V3 & V5 binding implementations for the Paho and HiveMQ client libraries.
Closes #335