-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding JSON schema for pointcloud extension #861
Conversation
No worries @mccarthyryanc - my time to work on this has been less than zero, so it’s great to see someone else jump in! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see someone working on this to complete the extension schemas!
I reviewed the Schema and request some changes. There are some more general questions regarding the extension included, maybe one of you can also improve these points. Maybe @hobu can give some additional information on how the things were meant to be?
Oh and after those changes, maybe run the whole schema through https://jsonformatter.org/ for consistency.
…pdating pointcloud json schema based on PR comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks. Only thing you could still change is #861 (comment)
Co-authored-by: Matthias Mohr <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, thanks!
Noticed there hadn't been any updates on this for a few months and thought I would give it a try (Sorry @adamsteer. If you already have something put together, I'm happy removing this PR.) I'm new to JSON schema so happy to get feedback.
One thing that I wasn't quite sure about was adding Property Dependencies on Statistics. So if a
stats
object is given, there should also be aschema
object with the same name.Related Issue(s): #391
Proposed Changes:
PR Checklist: