Skip to content

Conversation

@emmmile
Copy link

@emmmile emmmile commented Dec 9, 2015

Some modifications on top of the work from @jamesaimonetti (mostly added local URI decoding and support for refs to array elements).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Elvis:

Invalid macro name schema_loader_fun on line 49. Use UPPER_CASE.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Elvis:

Missing space after "," on line 263

@andreineculau
Copy link
Member

oh wow :) now I have 3 PRs (this, #5 and #2) to look into around $ref

@emmmile @jamesaimonetti @emfa - could I kindly ask of you to provide some input on one another? without looking at all into the PRs, @emmmile or @emfa (both building on top of @jamesaimonetti) could get some inspiration from each other (completeness, correctness, etc), and update this PR or #5, so that I close the other two.

NOTE: @emfa's work in #5 builds up on #4 as well, so accepting #5 would maybe make things easier. I'm not hinting at anything, nor being biased, just stating the less obvious.

@emmmile
Copy link
Author

emmmile commented Dec 9, 2015

@andreineculau actually you are right, sorry for having not checked #5 before.

#5 seems to cover all the cases, and it passes all the tests, so I guess that is the important thing.

There is a lot of additional code for draft 04, but for what concerns draft 03 the work of @emfa looks good to me. So I would close this one, and work on #5.

@andreineculau
Copy link
Member

thanks @emmmile!
closing in favour of #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants