Skip to content
Please note that GitHub no longer supports your web browser.

We recommend upgrading to the latest Google Chrome or Firefox.

Learn more
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

New translator for Cowles Foundation publications #2061

Open
wants to merge 18 commits into
base: master
from

Conversation

@placardo
Copy link
Contributor

placardo commented Nov 21, 2019

A translator for the documents on the Cowles Foundation website (https://cowles.yale.edu/).
The translator works on the authors' pages and on the different lists of publications (for monographs, discussion papers and published articles), and gets pdfs when available.
I tried to follow the guidelines at https://www.zotero.org/support/dev/translators.

Copy link
Collaborator

adam3smith left a comment

Thanks!
Some suggestions to clean up the code a bit & make it more robust

Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
Cowles.js Outdated Show resolved Hide resolved
placardo added 3 commits Nov 22, 2019
@placardo

This comment has been minimized.

Copy link
Contributor Author

placardo commented Nov 22, 2019

Thanks!
Some suggestions to clean up the code a bit & make it more robust

Thank you for your quick feedback! This is my first translator and I'm not used to Github so I apologize in advance for any silly mistakes

@zuphilip zuphilip requested a review from adam3smith Nov 25, 2019
@adam3smith

This comment has been minimized.

Copy link
Collaborator

adam3smith commented Dec 5, 2019

This all looks very good now thanks (and apologies for the delay).
Two remaining issues:

  1. The root that you specify -- that shouldn't be necessary. Zotero can handle relative links and they're more robust. Could you test if all of this still works entirely without root and then remove it? (If you're used to other webscraping tools e.g. in R or python, they tend to behave less like a browser and break with relative links; Zotero doesn't)

  2. Please use the minimized version of text() and attr() as e.g. here: https://github.com/zotero/translators/blob/master/Emerald%20Insight.js#L37 use all three lines -- the first one identifies, the 2nd disables linting, the 3rd is the minimized code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.