search

apiaryio / mson-ast

MSON AST Serialization Media Types
MIT License
4 stars 1 forks source link

MSON AST reflecting the new specification #3

Closed zdne closed 9 years ago

zdne commented 9 years ago

For @fosrias, @honzajavorek and @pksunkara to review.

fosrias commented 9 years ago

@zdne Done making comments. Looks great!

honzajavorek commented 9 years ago

I'm done too! The mind map image could have more colours, but it's good enough for now.

pksunkara commented 9 years ago

We should use camelCase.

pksunkara commented 9 years ago

The more I think, the more I am convinced that Named Type should probably inherit or should be same as Property Member.

zdne commented 9 years ago

camelCase it is

zdne commented 9 years ago

The more I think, the more I am convinced that Named Type should probably inherit or should be same as Property Member.

Please elaborate.

zdne commented 9 years ago

I have hopefully added more clarity to the AST map (however the principal layout does stay the same):

ast map

zdne commented 9 years ago

@honzajavorek @fosrias per your request I have extended the JSON AST example

fosrias commented 9 years ago

The more I think, the more I am convinced that Named Type should probably inherit or should be same as Property Member.

@zdne @pksunkara I disagree. Property Member inherits from Value member which would then imply a Named Type would have a value and an inline description. I think it is correct as it is.

zdne commented 9 years ago

@fosrias @honzajavorek @pksunkara I am done with fixes based on your review remarks. If there are no further remarks.

I would like @honzajavorek or @pksunkara to merge this PR (as they need to be familiar with the MSON AST the most).

Thank you!

fosrias commented 9 years ago

@zdne One typo I noted. After that fine by me for @honzajavorek or @pksunkara to merge.

honzajavorek commented 9 years ago

Also spotted "fot", otherwise good.

pksunkara commented 9 years ago

@zdne @fosrias @honzajavorek The MSON AST types are circular.

Type Section needs Member Type which needs Value Member which needs Type Section.

I can use a forward declaration in C++, but just wanted to make sure you guys know this.

zdne commented 9 years ago

I can use a forward declaration in C++, but just wanted to make sure you guys know this.

Yes, I am aware of this (read: this is intended). I am not sure whether a forward declaration is the way to go but this is not the right place to discuss the C++ implementation. We can do it in person tomorrow, or lets do it over a gist or a PR proposal.