Update annotation.rb #70
Conversation
| end | ||
|
|
||
| def string_only_keys | ||
| super + %w{ on } | ||
|
|
There was a problem hiding this comment.
Looks like you're missing an end for this method. You aren't by chance a Python programmer are you 😉 ?
There was a problem hiding this comment.
Ooops!
I'm actually more of a Perl programmer, but I was working on Python earlier in the day, so maybe that was it!
Actually, looking at my local copy, I've got the "end" in there. I didn't copy and paste so my head must have still been in Python mode ;)
There was a problem hiding this comment.
Added another commit to fix that missing "end".
I'm using the Github browser editor out of laziness/a desire for speed, but I admit I'm not so familiar with the web interface. Used to doing everything on the CLI...
|
Anything else I need to do to move this one along? |
|
Still interested in getting this one if possible. |
|
The build is failing and it looks like it might be a remote resource at princeton has moved to a different location. Maybe try rebasing this against master? |
|
Apologies for the delay, @jcoyne. However, it looks like it isn't behind master. The last commit in master is from January 2017, and my 2 commits are from March 2018. If the build is failing, I think that's actually a pre-existing problem in master. |
|
Looking at https://travis-ci.org/iiif-prezi/osullivan/builds, it says build passing... but I don't think there's been a build on master since January 2017... |
|
Could I get someone to look at the CI build? I'm pretty sure my code isn't the issue. I think there's a pre-existing issue in the master codebase. |
|
@minusdavid I think @jcoyne is right about the priceton resource change causing the problem. I updated the specs to use the new address and my build just passed. See pr #73 |
|
@minusdavid Conor fixed the pre-existing problem, so if you would rebase on master, the tests will hopefully pass. |
|
Cheers folks. I'll rebase and give it a go. |
|
Ah I think maybe I was/am confused about the proper branch for this to go into... what do you develop against? The development branch? |
According to http://iiif.io/api/presentation/2.0/#image-resources, "the URI of the canvas must be repeated in the on field of the Annotation". This patch adds the "on" field to the annotation class and makes it a required field. It's worth noting that without this patch, https://github.com/IIIF/presentation-validator will fail the validation for an O'Sullivan generated manifest that contains an annotation. With the patch, it will succeed.
minusdavid
force-pushed
the
minusdavid-patch-1
branch
from
90170b0
to
3772304
Compare
|
Hurray! Thanks again. Apologies for the delay on my end. |
|
Any chance of us getting this one in? Without it, the IIIF manifests fail when they go through a validator. |
|
Still waiting on this one. Is this project dead now? |
|
Is anyone monitoring this repository anymore? I'm thinking that I might end up forking and just updating locally, as it doesn't seem like this is maintained anymore? |
|
I'm noticing lots of commits from @cbeer. Would it be possible to get this pull request merged? I'm curious about the direction of the project as well, as it would be great to see support for new versions of the IIIF Presentation API as well... |
According to http://iiif.io/api/presentation/2.0/#image-resources, "the URI of the canvas must be repeated in the on field of the Annotation". This patch adds the "on" field to the annotation class and makes it a required field.
It's worth noting that without this patch, https://github.com/IIIF/presentation-validator will fail the validation for an O'Sullivan generated manifest that contains an annotation. With the patch, it will succeed.