Skip to content
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

W-11410812 - Support in draft 2019 as declarations key along with #1495

Merged
merged 2 commits into from
Jul 19, 2022

Conversation

looseale
Copy link
Contributor

No description provided.

val foundDefKeys = definitionsKeys.flatMap(map.key(_))
if (foundDefKeys.size > 1) { // If there is more than 1 definition key present in the map
// TODO add test in the new validation suite
ctx.eh.warning(
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going to parse only one key when both are presents, maybe should be a violation instead of a warning?
I think when some information is lost (not parsed) we should throw violations, because some information is not evaluated (and those ignored ast may be invalid at amf)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are parsing all the keys (or at least I suppose I do that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

definitionsKeys.flatMap(defKey => parseDeclarationMap(map, defKey, declarationKeysHolder)).toList
All the declarations in all the keys should be parsing

pope1838
pope1838 previously approved these changes Jul 19, 2022
@looseale looseale merged commit a8a2812 into develop Jul 19, 2022
@looseale looseale deleted the W-11410812/multi-defs branch July 19, 2022 20:50
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