daisy / pipeline-scripts

!! NOTE: This project is now part of the pipeline-modules project !! | Script modules for the default DAISY Pipeline 2 distribution.
GNU Lesser General Public License v3.0
6 stars 5 forks source link

DAISY 2.02 Validator #49

Closed josteinaj closed 9 years ago

josteinaj commented 10 years ago

This is a port of the "Daisy 2.02 DTB Light Validator" included in Pipeline 1. This implementation does not yet support validation of the MP3 files themself.

I've developed this on the same branch as the EPUB3 to DAISY 2.02 script, which is why the commit log contains some of those commits. However, this PR only contains the DAISY 2.02 Validator script and can be merged independently. The EPUB3 to DAISY 2.02 script will be in a separate PR.

rdeltour commented 10 years ago

A couple questions after skimming through the PR:

I'll add a couple detailed comments inline too.

Otherwise looks very good, thx!

josteinaj commented 10 years ago

Thanks for the detailed review.

are you OK if I squash all commits in a single one

sure

can you provide a summary of what difference there is with the ported validator ?

I tried replicating the behavior of the Pipeline 1 script as closely as possible. But I can go through and compare it and give more details.

rdeltour commented 10 years ago

But I can go through and compare it and give more details.

If you have the time, that w/b great indeed. People tend to ask these kind of things over time, e.g. when they need to decide whether they can move a production tool chain based on Pipeline 1 into Pipeline 2.

For now, I'll squash the commits, do the minor edits we mentioned inline, and accept the PR.

josteinaj commented 10 years ago

I added a few corrections in comments that should be included before merging.

I tried comparing with the DP1 code but I didn't manage to understand what it did. I remember spending quite some time in Eclipse trying to decrypt what the script did when I ported it, but I've forgot how it was right now. I've gone through validate.xpl now and made this overview though, do you think it is sufficient?

Validations made in the DAISY 2.02 Validator Pipeline 2 script:

rdeltour commented 10 years ago

I added a few corrections in comments that should be included before merging.

OK, will do.

I've gone through validate.xpl now and made this overview though, do you think it is sufficient?

Yeah, that's good enough. Thanks! I'll try to see if I can get some time to compare Pipeline 1's behavior against this list.

rdeltour commented 10 years ago

I squashed commit and then applied fixes to another commit for easier review. @josteinaj please review the fixes, I'll squash them and merge the PR if OK.

josteinaj commented 10 years ago

Didn't have time to test it until now, sorry.

The validator seems to work fine in oXygen, but I don't get any results from the framework...

rdeltour commented 9 years ago

Rebased, squashed and merged as 2b04ed2

I noticed a bug when testing, due to the HTML5 parser injecting some attributes to deprecated HTML elements (e.g. some a elements with no href get injected a shape attribute). I'll commit the fix in a separate PR.