add annotator - now with storage :-)

[devcurmudgeon]

No description provided.

[kinnison]

Have you given thought yet to where the annotations would be stored? Mustard does not "own" the git repository it typically reads from, so we'd need some kind of look-aside storage somewhere.

[Jannis]

The annotator licence is different from AGPLv3, so we should add a COPYING.annotator with its complete licence text, plus a hint at the end of README.md, like we did for jQuery.

[devcurmudgeon]

On 09/02/2014 10:12, Daniel Silverstone wrote:

Have you given thought yet to where the annotations would be stored?
Mustard does not "own" the git repository it typically reads from, so
we'd need some kind of look-aside storage somewhere.

Ideally they would end up in the git repo somehow, but if that's not
practical a short term solution would maybe be an sqlite database. But
you guys are better placed to choose that, I think. Probably worth
looking at the annotator storage examples for ideas.

For Mustard specifically, we're not talking about hyperscale
interactions - just reviews by team members and customers/users. I think
this initial use-case doesn't really need to deal with threaded
discussions, collisions etc. A simplistic solution with persistence
would already be a great improvement, imo.

[devcurmudgeon]

On 09/02/2014 11:56, Jannis Pohlmann wrote:

The annotator licence is different from AGPLv3, so we should add a
COPYING.annotator with its complete licence text, plus a hint at the end
of README.md, like we did for jQuery.

OK, I've done that in the branch

[devcurmudgeon]

I've managed to get the persistent storage working, and enabled markdown support in the annotations

[Jannis]

I looked at the code changes. You're hard-coding 127.0.0.1:5000 in the JavaScript part. That's probably not going to work because the JavaScript is executed client-side and you can't expect all users to have Elasticsearch enabled. It also means that you'd have a per-user store, not store on the server shared by all users. You could try to determine the host domain from the URL of the current document. It still assumes the Elasticsearch store and Mustard are running on the same server but that should be ok for now.

I don't see any instructions in README.md on the annotations feature, nor on how Elasticsearch is to be set up on the server-side.

Not quite ready for merging IMHO.

[Jannis]

I also don't see the need to limit fetching annotations per URL to only 20. That number will soon be exceeded and then we'll have to increase it. How about a limit like 1000 or no limit at all until we encounter performance issues?

[devcurmudgeon]

On 10/02/2014 17:26, Jannis Pohlmann wrote:

I also don't see the need to limit fetching annotations per URL to only
20. That number will soon be exceeded and then we'll have to increase
it. How about a limit like 1000 or no limit at all until we encounter
performance issues?

I just copied that directly from the Annotator example - your
suggestions are clearly better

[devcurmudgeon]

On 10/02/2014 17:22, Jannis Pohlmann wrote:

I looked at the code changes. You're hard-coding 127.0.0.1:5000 in the
JavaScript part. That's probably not going to work because the
JavaScript is executed client-side and you can't expect all users to
have Elasticsearch enabled. It also means that you'd have a per-user
store, not store on the server shared by all users. You could try to
determine the host domain from the URL of the current document. It still
assumes the Elasticsearch store and Mustard are running on the same
server but that should be ok for now.

I don't see any instructions in README.md on the annotations feature,
nor on how Elasticsearch is to be set up on the server-side.

I agree with your points, but won't be able to work on the changes any
time soon. I'd be happy for someone else to pick this up.

[devcurmudgeon]

OK, I've rebased this against master, and fixed the hardcoded url - it's working here for me...