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

fix(schema-documentation): update schemas and documentation (DEV-61) #105

Merged
merged 22 commits into from Oct 12, 2021

Conversation

@irinaschubert
Copy link
Collaborator

@irinaschubert irinaschubert commented Oct 4, 2021

resolves DEV-61
resolves #43
resolves #38
resolves Pulldown/List issue for listValues --> only List allowed

@irinaschubert irinaschubert self-assigned this Oct 4, 2021
@irinaschubert irinaschubert marked this pull request as ready for review Oct 4, 2021
@irinaschubert irinaschubert requested a review from Vijeinath Oct 4, 2021
@irinaschubert
Copy link
Collaborator Author

@irinaschubert irinaschubert commented Oct 4, 2021

Hi @Vijeinath , in this PR I integrated the changes Stefan has reported. I updated the schemas too as there were quite some inconsistencies. As Balduin doesn't work this week, I thought you could have a look at it instead.

Loading

docs/dsp-tools-create.md Outdated Show resolved Hide resolved
Loading
docs/dsp-tools-create.md Outdated Show resolved Hide resolved
Loading
docs/dsp-tools-create.md Outdated Show resolved Hide resolved
Loading
docs/dsp-tools-create.md Outdated Show resolved Hide resolved
Loading
docs/dsp-tools-create.md Outdated Show resolved Hide resolved
Loading
Copy link
Contributor

@Vijeinath Vijeinath left a comment

There are just small issues

Loading

@irinaschubert irinaschubert requested a review from Vijeinath Oct 5, 2021
},
"ncname": {
"type": "string",
"pattern": "^[a-zA-Z0-9][a-zA-Z0-9_-]*$",

Choose a reason for hiding this comment

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

@irinaschubert According to https://www.w3.org/TR/xml11/#NT-NameStartChar, NCName is not allowed to start with a number (but with underscore). Would that be valid for the API?

Loading

Copy link
Collaborator Author

@irinaschubert irinaschubert Oct 12, 2021

Choose a reason for hiding this comment

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

That's right, the pattern wasn't correct. I changed it according to your suggestions and I also added "." as allowed character.

Loading

Copy link
Contributor

@Vijeinath Vijeinath left a comment

I'm happy

Loading

@@ -59,7 +57,7 @@
]
},
{
"pattern": "^([\\w-]*)?:(\\w+)$"
"pattern": "^([-\\w]*)?:(\\w+)$"
Copy link

@musicEnfanthen musicEnfanthen Oct 5, 2021

Choose a reason for hiding this comment

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

Would it make sense to create a definition for this pattern (like prefixedname)? It seems to be used quite often. Is it correct that the first group sometimes ends with * and sometimes with + ?

Loading

Copy link
Collaborator Author

@irinaschubert irinaschubert Oct 12, 2021

Choose a reason for hiding this comment

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

Yes, good idea. I introduced prefixedname for this pattern and replaced its occurrences. I also unified the use of * and +.

Loading

"isPartOf",
"seqnum"
]
},
{
"pattern": "^(([\\w-]*)?:([\\w ]+))$"
"pattern": "^(([-\\w]*)?:([\\w ]+))$"

Choose a reason for hiding this comment

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

@irinaschubert white space correct here?

Loading

Copy link
Collaborator Author

@irinaschubert irinaschubert Oct 12, 2021

Choose a reason for hiding this comment

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

No, it's not. I used prefixedname instead.

Loading

@irinaschubert irinaschubert merged commit 4d9c1e4 into main Oct 12, 2021
2 checks passed
Loading
@irinaschubert irinaschubert deleted the wip/dev-61-fix-documentation branch Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants