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

support other CRAN mirrors #1552

Merged
merged 8 commits into from Feb 27, 2018

Conversation

@katrinleinweber
Copy link
Contributor

commented Feb 8, 2018

Hi!

Since CRAN.RStudio.com is a very popular mirror, please consider expanding the translator to it. Related to #1487.

Also, would a minimal reg-ex to cover even other mirrors be advisable? cran\..+/web/packages might work.

Cheers :-)

katrinleinweber added some commits Feb 8, 2018

@katrinleinweber
expand to CRAN mirror
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…
7952242
@katrinleinweber
add test
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…
c50bebf
@adam3smith

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Feb 8, 2018

Generally happy with this & extending it to other mirrors. Note you'll have to bump the timestamp for updates to propagate (Scaffold, which we recommend, would do that automatically)
@adam3smith

adam3smith reviewed Feb 8, 2018
View changes

R-Packages.js Outdated
@@ -2,7 +2,7 @@
"translatorID": "24a10ebf-ada1-4b8d-8f76-5a29e24d3e78",
"label": "R-Packages",
"creator": "Sebastian Karcher",
"target": "r-project\\.org/web/packages/(.+/index\\.html|available_packages_by_(name|date)\\.html)",
"target": "(r-project\\.org|rstudio\\.com)/web/packages/(.+(/index\\.html)?|available_packages_by_(name|date)\\.html)",

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 8, 2018

Collaborator

once you make index.html optional, you can just delete the entire regex after packages: .+(/index\\.html)? literally matches anything.
@katrinleinweber
ensure update propagates
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…
ca90d7a
@adam3smith

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Feb 8, 2018

What's the form of the other mirrors? I thought we already covered those?
@katrinleinweber

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Feb 8, 2018

Oh! That was an unwarranted assumption it seems. I was thinking of https://www.r-pkg.org/pkg/ggplot2 but they seem to have an entirely different site structure.

katrinleinweber added some commits Feb 8, 2018

@katrinleinweber
expand to more CRAN mirrors
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…
25a5eff
@katrinleinweber
prevent too promiscuous matching
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…
90b2fa1
@katrinleinweber

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Feb 8, 2018

I used https://duckduckgo.com/?q=cran+web+packages and https://www.google.de/search?q=cran+web+packages to find other popular mirrors.

https://mran.microsoft.com/ & https://www.r-pkg.org/ are out-of-scope for me currently.

@katrinleinweber katrinleinweber changed the title expand to CRAN mirror support other CRAN mirrors Feb 8, 2018

@katrinleinweber

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Feb 8, 2018 •
edited

I feel bad duplicating the test code so much. Does Scaffold support extracting multiple URLs and comparing each result against the same items set? Or against each other?
@adam3smith

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Feb 8, 2018

Don't worry about it -- we only very rarely test different sites with the same expected result, so not worth implementing dedicated functionality for that scenario, and the size of these tests is just completely negligible (and it's not like it's code that actually needs to be updated/maintained by humans)
@katrinleinweber

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Feb 8, 2018

I delivered it with my bare hands, but it's good to know I don't have to take care of it. 🤣

@katrinleinweber katrinleinweber referenced this pull request Feb 9, 2018

Open

How to deal with CRAN mirrors? #5

@adam3smith
Add more examples, lower priority 
Priority as per #765
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…
78aa8d1
@katrinleinweber

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Feb 12, 2018

related to force11/force11-sciwg#9
@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Feb 18, 2018

I rewrote it w/o the FW and update some things. Please have a look. Should be fine for merge.
@katrinleinweber

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Feb 19, 2018

Manual test imports look fine for me (Zotero 5.0.35.1 & Firefox 52.6.0). Thank you :-)
@adam3smith

adam3smith reviewed Feb 27, 2018
View changes

R-Packages.js Outdated
"creatorType": "author"
},
{
"firstName": "R. Core",

This comment has been minimized.

Sign in to view
Copy link
@adam3smith

adam3smith Feb 27, 2018

Collaborator

we should recognize the R Core Team and turn it into an institutional author. Presumably that'll show up on a fair number of packages, so should be worth it.

This comment has been minimized.

Sign in to view
Copy link
@zuphilip

zuphilip Feb 27, 2018

Collaborator

Done in the new version.
@katrinleinweber

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Feb 27, 2018

Thank you :-)

@zuphilip zuphilip merged commit 24a2462 into zotero:master Feb 27, 2018
1 check passed

1 check passed

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

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Feb 27, 2018

Okay, it is merged now. Thank you all for the contributions here!
@zuphilip

This comment has been minimized.

Sign in to view
Copy link
Collaborator

commented Feb 27, 2018

@katrinleinweber Can we close the corresponding issue or is there something still open? (I haven't followed the conversion there closely.)
@katrinleinweber

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Feb 27, 2018 •
edited

#1487 is "related", because it deals with CRAN iFrames. It's not completely corresponding to this PR, so it should IMHO stay open.

@katrinleinweber katrinleinweber referenced this pull request Jan 24, 2019

Open

collaboration with Zotero #91

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adam3smith adam3smith

@zuphilip zuphilip

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@katrinleinweber @adam3smith @zuphilip
You can’t perform that action at this time.