Skip to content

/translators

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup some old code #1537

Merged
merged 15 commits into from Mar 15, 2018

Conversation

Reviewers

@adam3smith adam3smith

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@zuphilip @adam3smith
@zuphilip
Copy link
Collaborator

zuphilip commented Jan 28, 2018

These are in the list of translators still using wait, which I generated with

grep -L "/* FW LINE" *.js | xargs -d '\n' grep "Zotero.wait("

TODO: Reuters might still need a little more work to make sure that the test case for the newspaperArticle is working.

@zuphilip zuphilip force-pushed the zuphilip:cleanup-waits branch from 63eb9ed to c55d0e1 Jan 31, 2018

@zuphilip zuphilip force-pushed the zuphilip:cleanup-waits branch from 6d08b3a to 206f5e6 Feb 1, 2018

@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator Author

zuphilip commented Feb 1, 2018

( TODO: rename jmlr.js to the name of the label, but this will be detected as a deletion and shown as a fail in Travis test --> postponed )

zuphilip added some commits Feb 1, 2018

@zuphilip
Update FreePatentsOnline.js
Loading status checks…
b177302
@zuphilip
Delete KOBV.js old portal 
See http://vs13.kobv.de/V/?func=file&file_name=kobv-news#
for announcement of coming shutdown and link to new
portal.
Loading status checks…
af75140
@zuphilip
Rewrite Musee du Louvre.js
Loading status checks…
d4e3ac3
@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator Author

zuphilip commented Feb 11, 2018

Last commit solves #188 and #1206.

@zuphilip zuphilip force-pushed the zuphilip:cleanup-waits branch from d7470f8 to 45ba338 Feb 11, 2018

@adam3smith

This comment has been minimized.

Sign in to view
Copy link
Collaborator

adam3smith commented Feb 12, 2018

Do you want me to review & merge this?
@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator Author

zuphilip commented Feb 12, 2018

Do you want me to review & merge this?

Yes, that would be fine. There are some more cases, but I can pack them into a new PR.
@adam3smith

adam3smith requested changes Feb 27, 2018
View changes

Copy link
Collaborator

adam3smith left a comment

Looks good, thanks for doing this! A couple of small things to do quite right while you're cleaning up. Feel free to merge yourself once they're done.
Archeion.js Outdated
var translator = Zotero.loadTranslator("import");
translator.setTranslator("5e3ad958-ac79-463d-812b-a86a9235c28f");
translator.setString(text);
translator.setHandler("itemDone", function (obj, item) {
//the DC doesn't distinguish between personal and institutional authors - get them from the page and parse
var authors = ZU.xpath(doc, '//div[@id="archivalDescriptionArea"]//div[@class="field"]/h3[contains(text(), "Name of creator")]/following-sibling::div/a');
for (var i in authors) {
for (let i in authors) {

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 27, 2018

Collaborator

You're still using for ... in to cycle through an array, though...
Archeion.js Outdated
//remove location (in parentheses) from creators, since it often contains a comma that messes with author parsing
item.creators[i] = ZU.cleanAuthor(authors[i].textContent.replace(/\(.+\)\s*$/, ""), "author", true);
if (!item.creators[i].firstName) item.creators[i].fieldMode = 1;
}
//The Archive gets mapped to the relations tag - we want its name, not the description in archeion
for (var i in item.seeAlso) {
for (let i in item.seeAlso) {

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 27, 2018

Collaborator

same here
Code4Lib Journal.js Outdated
}

newItem.attachments.push({
url:doc.location.href,

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 27, 2018

Collaborator

wouldn't document: doc, be better here?
FreePatentsOnline.js Outdated
function getSearchResults(doc, checkOnly) {
var items = {};
var found = false;
// TODO: adjust the CSS selector

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 27, 2018

Collaborator

clean up TODOs here
FreePatentsOnline.js Outdated


newItem.attachments.push({
doc: doc,

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 27, 2018

Collaborator

I think that needs to be document: doc,
Musee du Louvre.js Outdated
"url": "http://www.louvre.fr/en/oeuvre-notices/mona-lisa-portrait-lisa-gherardini-wife-francesco-del-giocondo?sous_dept=1",
"creators": [
{
"lastName": "Leonardo di ser Piero da Vinci, known as LEONARDO DA VINCI (Vinci, 1452 - Amboise, 1519)",

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 27, 2018

Collaborator

This is never going to be perfect, but could we at least remove the date of birth&death in parentheses?
Reuters.js Outdated
@@ -164,56 +184,55 @@ var testCases = [
"creatorType": "author"
}
],
"notes": [],
"date": "Mon Nov 14 21:16:28 UTC 2011",

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 27, 2018

Collaborator

I'm worried how Zotero is going to parse that date. We should normalize that or remove the timestamp
@zuphilip
Update Clean-ups
Loading status checks…
b4bb65a
@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator Author

zuphilip commented Mar 5, 2018

Okay, I updated them, but either you or me should look over them again. I will not able to do this very soon...

adam3smith added some commits Mar 15, 2018

@adam3smith
Use getSearchResults for Code4lib detect
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
9c8c75e
@adam3smith
Tagesspiegel tag clean-up improvements
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
ee9a56b
@adam3smith

adam3smith approved these changes Mar 15, 2018
View changes

@adam3smith adam3smith merged commit 5bb111a into zotero:master Mar 15, 2018
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@adam3smith

This comment has been minimized.

Sign in to view
Copy link
Collaborator

adam3smith commented Mar 15, 2018

Awesome -- thanks!

@zuphilip zuphilip deleted the zuphilip:cleanup-waits branch Mar 16, 2018

@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator Author

zuphilip commented Mar 16, 2018

Thank you for the review and improvements!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.