Finished JSON Schema serialization #552
Conversation
tjanc
commented
Jun 8, 2018
•
- adds support for Object
- adds support for nullable
- fixes some inconsistencies
- hooks in new serialization
721e515 to
e3ff353
Compare
| auto it = find(str->get().get()); | ||
| if (it != end()) | ||
| erase(it); | ||
| } |
There was a problem hiding this comment.
This cascade is little bit confusing while reading
There was a problem hiding this comment.
It just checks whether we are adding a MemberElement with StringElement key so we ensure uniqueness - if the key is already encountered, it shall be removed. Specific suggestions how to rewrite this?
|
Great work! But would you please split all changes into small commits? E.g. you can extract refactoring staff like this https://github.com/apiaryio/drafter/pull/552/files#diff-b3cd98f0ba049e7bdc9ae6c68e9e1345L17 into separate commit. I think small commits will be much more easier to check and understand. |
| @@ -57,10 +57,6 @@ void MarkdownParser::parse(const ByteBuffer& source, MarkdownNode& ast) | |||
| m_source = NULL; | |||
| m_sourceLength = 0; | |||
| m_listBlockContext = false; | |||
|
|
|||
| #ifdef DEBUG | |||
There was a problem hiding this comment.
Because the dump there is
a) pretty hacky
b) messes up standard output
c) useless.
There was a problem hiding this comment.
Unless we have an alternative, please don't remove it. It's pretty useful when debugging.
There was a problem hiding this comment.
I'm still convinced writing debug information to standard output out of nowhere is a bit too hacky. You can always log like this:
LOG(debug) << "Hello " << 42 << " or some other value";
This amounts to the same thing but can be dynamically turned on with a flag: drafter -L or drafter --enable-log. Also, it logs into the file drafter.log instead of polluting standard output. If needed, the ENABLE_LOGGING macro can be used to force logging while debugging.
There was a problem hiding this comment.
This shall be solved in a future PR, for now I'm reverting the change.
|
@tjanc can you please rebase this on master so it will trigger the new CircleCI tests. |
60c2638 to
cfbc4d6
Compare
cfbc4d6 to
81573ee
Compare