-
Notifications
You must be signed in to change notification settings - Fork 3
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
First draft of RNG and XML #19
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good overall!, though see my two comments. And I have two additional comments:
- do we need the RNG in addition to the RNC? if not, we could just keep the RNC.
- maybe config.xml should be named config-example.xml
rng/config.xml
Outdated
@@ -0,0 +1,17 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> | |||
<?xml-model href="file:/local/repos/GH/sknorr/dietrich/rng/conversion.rnc" type="application/relax-ng-compact-syntax"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you shouldn't commit that line. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! 👍
rng/config.xml
Outdated
<?xml version="1.0" encoding="UTF-8"?> | ||
<?xml-model href="file:/local/repos/GH/sknorr/dietrich/rng/conversion.rnc" type="application/relax-ng-compact-syntax"?> | ||
<config version="1.0"> | ||
<property key="STLYEROOT" value="/usr/share/xml/docbook/stylesheet/suse2013"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using a style more like <styleroot>/usr/...</styleroot>
for all the properties would make this easier to validate imo. (e.g. see your STLYEROOT typo above that we can't catch currently)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, integrated.
See #18