eclipse-uprotocol / up-java

uProtocol Language Specific Library for Java
Apache License 2.0
9 stars 14 forks source link

Check ranges for Micro URI and add missing test coverage #97

Closed stevenhartley closed 6 months ago

stevenhartley commented 7 months ago

76

PLeVasseur commented 7 months ago

Thanks for taking this on! :slightly_smiling_face:

I'm going to approach this as if you're actually addressing issue #94 as it's a superset of #76

Noticed a couple of things we still need to check:

Also some general feedback -- I think it'd be a little cleaner to separate the validation logic out from the serialization itself.

For reference, here's the PR from up-rust which did this change.

(edited to add some references to the relevant code sections)

PLeVasseur commented 6 months ago

@stevenhartley, @neelam-kushwah -- I left some feedback in this comment which was not addressed.

Is the plan to make a follow-up PR?