Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upUpdate AMS MathSciNet.js target (#1451) #1452
Conversation
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
adam3smith
Oct 20, 2017
Collaborator
Thanks, this looks great. It also looks like the page now defaults to https, so we should update that (it's already covered in the target, but needs updating on line 100 and in the tests)
Thanks, this looks great. It also looks like the page now defaults to https, so we should update that (it's already covered in the target, but needs updating on line 100 and in the tests) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
gjankowiak
Oct 20, 2017
Contributor
I can take care of that, but not for the test going through libezproxy2.syr.edu, I don't have credentials and the certificate there doesn't match the subdomains. Shall I leave it out?
I can take care of that, but not for the test going through libezproxy2.syr.edu, I don't have credentials and the certificate there doesn't match the subdomains. Shall I leave it out? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
adam3smith
Oct 20, 2017
Collaborator
That's a mistake on my part, actually. It should never have gone into the test. Just delete that bit from the test while you're at it please.
That's a mistake on my part, actually. It should never have gone into the test. Just delete that bit from the test while you're at it please. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
gjankowiak
Oct 20, 2017
Contributor
Ok, will do. I will leave the HTTP/HTTPS alternative in the target, it shouldn't hurt.
Ok, will do. I will leave the HTTP/HTTPS alternative in the target, it shouldn't hurt. |
"items": "multiple" | ||
}, | ||
{ | ||
"type": "web", | ||
"url": "http://www.ams.org.libezproxy2.syr.edu/mathscinet-getitem?mr=2767535", |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
zuphilip
Oct 21, 2017
Collaborator
I wouldn't delete this test completely but rather change the url to a non-proxified url, i.e. https://mathscinet.ams.org/mathscinet-getitem?mr=2767535 . Otherwise this looks good.
zuphilip
Oct 21, 2017
Collaborator
I wouldn't delete this test completely but rather change the url to a non-proxified url, i.e. https://mathscinet.ams.org/mathscinet-getitem?mr=2767535 . Otherwise this looks good.
adam3smith
added some commits
Oct 21, 2017
adam3smith
merged commit 3e9d86c
into
zotero:master
Oct 21, 2017
1 check passed
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Thank you! |
gjankowiak
deleted the
gjankowiak:patch-1
branch
Oct 23, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Thank you for fixing this quickly! |
gjankowiak commentedOct 20, 2017
changed initial subdomain from www. to mathscinet.. Also updated test cases.
Fixes #1451.