search

metanorma / mnconvert-ruby

Ruby gem wrapper for mnconvert
2 stars 0 forks source link

Fix/ieee xml #12

Closed opoudjis closed 2 years ago

opoudjis commented 2 years ago

Metanorma PR checklist

opoudjis commented 2 years ago

This is waiting on https://github.com/metanorma/mnconvert/issues/195

I need for you to get this working with validation_against: standards.dtd, where standards.dtd is in the folder

"https://github.com/metanorma/ieee-stl/blob/main/standards-1-7-DTD/standards-1-7-dtd/standards.dtd"

CAMOBAP commented 2 years ago

For this particular case as far as I understand DTD comes from the private repo. So it doesn't make sense to implement URL validation on mnconvert.jar side

I think we can add CI-only tests (because CI token can download private repos) so we can do DTD validation tests on CI only

@ronaldtse how do you think will it be worth it?

opoudjis commented 2 years ago

It's not mandatory to test IEEE XML validation, but I'd like for it to happen. Need this resolved for https://github.com/metanorma/metanorma-ieee/issues/132

ronaldtse commented 2 years ago

For this particular case as far as I understand DTD comes from the private repo. So it doesn't make sense to implement URL validation on mnconvert.jar side

I think we can add CI-only tests (because CI token can download private repos) so we can do DTD validation tests on CI only

@ronaldtse how do you think will it be worth it?

@CAMOBAP I agree with this approach of using the private DTD in the GHA jobs.

I have asked IEEE SA whether we can share the DTD publicly, but until they answer positively, we have to use this private GHA approach. Thanks!

ronaldtse commented 2 years ago

IEEE SA has answered that they want to keep the DTD private. So we will have to action the GHA approach. @CAMOBAP @Intelligent2013 can you please help facilitate? Thanks!

CAMOBAP commented 2 years ago

@ronaldtse I will work on it

CAMOBAP commented 2 years ago

Now DTD validation test ready, but the test fails because of validation issues

@opoudjis @Intelligent2013 what are the next steps?

Intelligent2013 commented 2 years ago 
.../rice-en.cd.isosts.xml is NOT valid reason:
...
.../rice-en.cd.ieee.xml is NOT valid reason:

But in https://github.com/metanorma/mnconvert/actions/runs/3084996936/jobs/4987787985 all checks ended ok.... Let me try to understand this...

Intelligent2013 commented 2 years ago

The most issues will be fixed in mnconvert v1.24, except one .../rice-en.cd.ieee.xml is NOT valid reason:, because it generated from ISO MN XML, not IEEE MN XML. And I didn't test mnconvert for IEEE XML generation from arbitrary MN non-IEEE XML. I'll try to update mnconvert.

Intelligent2013 commented 2 years ago

@CAMOBAP validation issues fixed.

ronaldtse commented 2 years ago

@Intelligent2013 but we're not supposed to generate IEEE XML from other flavors. Only MN IEEE docs can output IEEE XML. There is clearly some misunderstanding here somewhere?

ronaldtse commented 2 years ago

Anyway the validation passes now, and let's merge it.

But still we should not allow generating IEEE XML from other flavors. It would be a nightmare to support that.