search

phyloref / jphyloref

MIT License
0 stars 0 forks source link

Adds support for JSON-LD files with relative URIs #26

Closed gaurav closed 5 years ago

gaurav commented 6 years ago

JPhyloRef currently fails when the WebserverCommand is given a JSON-LD file with relative URIs. This PR resolves that by using a temporary base URI when loading the JSON-LD file, then removing the temporary URI when providing results back to the client.

Ready for review!

hlapp commented 6 years ago

Did you want me to review this?

hlapp commented 6 years ago

BTW please pay attention to your commit messages - the first line regularly exceeds the length that makes them behave well (recommendation is 50 chars max, don't go >60).

gaurav commented 5 years ago

I'll merge this once #29 has been resolved and merged, and once I've pulled from master to make sure it still works!

hlapp commented 5 years ago

Was I somehow tricked into thinking this is JS code? Either way, know that you don't have to accept suggestions, and if I make a suggestion that's almost but not quite right but that you want to otherwise accept, it's probably better to create the change from scratch, unless Github allows you to edit the suggestions before committing. (It's a new feature, so I'm actually not sure how it works exactly.)

gaurav commented 5 years ago

Github doesn't allow you to edit the suggestions before committing. I prefer accepting suggestions as written and then fixing them subsequently -- it maintains a record of how the suggestion has been modified over time, and with all our syntax checking tests I'd imagine a lot of suggestions will need minor fixes anyway. But yes, I'll reject it and make the change myself if it's obviously wrong -- and hopefully Github will make it possible to edit suggestions soon!

hlapp commented 5 years ago

Github doesn't allow you to edit the suggestions before committing.

Ah, I didn't realize that.