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

Add Edge Validation for directed field #2066

Open
d47853 opened this issue Feb 1, 2019 · 4 comments
Open

Add Edge Validation for directed field #2066

d47853 opened this issue Feb 1, 2019 · 4 comments

Comments

@d47853
Copy link
Member

@d47853 d47853 commented Feb 1, 2019

Validation should be added to directed fields in schemas. This will be done as part of work for version 2 as adding in validation would cause breaking changes.

@d47853 d47853 added this to the v2.0.0 milestone Feb 1, 2019
@n288TJYRX
Copy link
Contributor

@n288TJYRX n288TJYRX commented Feb 11, 2019

What is this blocked by? Is it okay to go ahead and work on?

@n288TJYRX
Copy link
Contributor

@n288TJYRX n288TJYRX commented Feb 13, 2019

Theres a method validateDirection() in SchemaElementDefinitionValidator.java that is never used and is commented out. This is within the develop branch already, so it looks like its already been merged. Shall I uncomment it and create a unit test for it?

@m55624
Copy link
Contributor

@m55624 m55624 commented Feb 20, 2019

So I wrote this method and added it in, however the reason this ticket is blocked and it is commented out is that it is a breaking change, as for users who have schemas without direction validation currently, it would throw an exception and require direction. We cannot make breaking changes on minor versions so it will have to wait until V2, however feel free to create a branch and re add this method back in, and add a unit test, and when we get to the point of V2 we can merge it back in.

EDIT - For more context see #1980

@m55624 m55624 added migration-required and removed blocked labels Feb 20, 2019
@n288TJYRX
Copy link
Contributor

@n288TJYRX n288TJYRX commented Mar 6, 2019

Ready for pull request once we get to V2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.