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

feat(composition): Relax @tag definition validation #1022

Merged
merged 6 commits into from
Sep 23, 2021

Conversation

trevor-scheer
Copy link
Member

Fixes #1021

Copy link
Contributor

@martijnwalraven martijnwalraven left a comment

Choose a reason for hiding this comment

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

I don't know if I have all the context, but implementing an immediate workaround for the lack of repeatable support in Kotlin definitely makes sense to me. I also think we want to be more lenient in allowing differences between directive definitions in general. I'd prefer if we came up with a design for that first though, and think through the consequences to make sure this is something we feel comfortable supporting for all directives.

Leaving out repeatable has no effect on the semantics of directive applications (except that there would never be more than one tag per element for that subgraph).

Additional arguments are a somewhat different case however, because we have no way of knowing what a user's intention is. So just ignoring them might be surprising or even dangerous. We may also want to differentiate between nullable and non-nullable arguments, or take default values into account.

Copy link
Contributor

@martijnwalraven martijnwalraven left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@trevor-scheer trevor-scheer self-assigned this Sep 23, 2021
@trevor-scheer trevor-scheer merged commit e65a873 into main Sep 23, 2021
@trevor-scheer trevor-scheer deleted the trevor/compatible-tag-definition branch September 23, 2021 18:15
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.

feat(composition): Relax @tag directive definition validation
2 participants