Skip to content

Conversation

DennisOng
Copy link

For: #271

Unfortunately, the syntax proposed here will require using array within the YAML draft file, which would be incompatible with: #260

Opening this for a discussion, not sure what would be the best way forward to be honest

@DennisOng
Copy link
Author

Ah sorry just saw the other PR from @nathane on this same thing, what a coincidence!

Feel free to close this off if that works :)

@jasonmccreary
Copy link
Collaborator

jasonmccreary commented Aug 27, 2020

Actually, per the comment I left on that PR, I like your syntax better for now. Is it possible for you to review that PR and combine any improves (style, code, test coverage) into this one?

Also, can you update your PR description with the new syntax.

Again, likely something I will change in v2. But the nested array structure is more inline with the current Blueprint syntax than underscores...

@jasonmccreary
Copy link
Collaborator

Thanks. I have updated this to avoid changing the public method signature of Blueprint::parse. Hopefully merging #351 shortly.

@DennisOng
Copy link
Author

@jasonmccreary Cheers for taking over and making those changes, good idea on limiting the check for array syntaxes, was stuck on the best way forward there.

Sorry for the delayed response to review the initial PR, was having a long overdue self disconnect --force weekend :)

Thanks

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.

2 participants