Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Implement /on/npm/foo pages #4151

Closed
wants to merge 12 commits into from
Closed

Implement /on/npm/foo pages #4151

wants to merge 12 commits into from

Conversation

@chadwhitacre
Copy link
Contributor Author

😮

screen shot 2016-10-20 at 3 26 34 pm

@chadwhitacre
Copy link
Contributor Author

"lol" —@kaguillera

screen shot 2016-10-20 at 3 33 29 pm

@chadwhitacre chadwhitacre mentioned this pull request Oct 21, 2016
23 tasks
@chadwhitacre
Copy link
Contributor Author

Rebased and squashed, previous head was f77da78.

@chadwhitacre
Copy link
Contributor Author

@chadwhitacre
Copy link
Contributor Author

I want to move the payment widget to above the content area, full width. The sidebar would then only be for second-level nav where that exists.

@chadwhitacre
Copy link
Contributor Author

[gratipay] $ echo '# Greetings, program!' | marky-markdown /dev/stdin
<h1 id="user-content-greetings-program" class="deep-link"><a href="#greetings-program">Greetings, program!</a></h1>
[gratipay] $

@chadwhitacre
Copy link
Contributor Author

Alright, it's not going to be performant enough to make a heavy JSON API request and shell out to render markdown on every request for an /on/npm/foo/ page.

@chadwhitacre
Copy link
Contributor Author

npm gives us a big download at http://registry.npmjs.org/-/all (200 MB). I'm going to explore what's in there and think about how we might use that.

(Pdb) pp npm['aspen']
{u'author': {u'email': u'[email protected]', u'name': u'Chad Whitacre'},
 u'description': u'A NodeJS web framework that makes the most of the filesystem.',
 u'dist-tags': {u'latest': u'1.1.0'},
 u'maintainers': [{u'email': u'[email protected]', u'name': u'whit537'}],
 u'name': u'aspen',
 u'repository': {u'type': u'git',
                 u'url': u'https://github.com/zetaweb/aspen.js.git'},
 u'time': {u'modified': u'2013-02-15T16:10:08.600Z'},
 u'versions': {u'1.1.0': u'latest'}}
(Pdb)

@chadwhitacre
Copy link
Contributor Author

That doesn't give us readme, but it gives us everything else we want: name, description, author, maintainers. If we shove that in a table, we can use time.modified to update, including refetching and reprocessing the readme.

@kaguillera
Copy link
Contributor

👍 because so far http://npmsearch.com/ does not seem to give you the flexibility to search on specific fields.
The search input is search on all of the fields that they you are allowed to return. At least that is what is seems so far

@aandis aandis mentioned this pull request Oct 21, 2016
2 tasks
@chadwhitacre
Copy link
Contributor Author

Alright, circled the wagons IRL and decided to try out a local npm table, since it'll help this (with the processed readmes), and also search (#4145), and also keying by email (to help with /on/npm/).

@chadwhitacre
Copy link
Contributor Author

Okay! Circling back here now that we have npm loaded locally. We have pages from local npm! Let's find an example with a readme synced ...

screen shot 2016-10-26 at 10 22 26 pm

@chadwhitacre
Copy link
Contributor Author

Hmmm ... we trust marky-markdown to give us safe HTML, ya? 😳

screen shot 2016-10-26 at 10 28 40 pm

@kaguillera
Copy link
Contributor

We will certainly need to verify that asap.

@chadwhitacre
Copy link
Contributor Author

@kaguillera Well, I suppose npm trusts it, and ... we trust npm, right?

@kaguillera
Copy link
Contributor

I guess

On Oct 27, 2016 5:53 PM, "Chad Whitacre" [email protected] wrote:

@kaguillera https://github.com/kaguillera Well, I suppose npm trusts
it, and ... we trust npm, right?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4151 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABNTSZ1unwXm6isZqDp3i6wE3VNxXH9Nks5q4R0CgaJpZM4KcgkY
.

@chadwhitacre
Copy link
Contributor Author

Rebased (was db4539b).

@chadwhitacre
Copy link
Contributor Author

I'm taking a backup so I can get processed readmes to work with.

@chadwhitacre
Copy link
Contributor Author

Aaaaaaaaaaah ... much better!

image

@chadwhitacre
Copy link
Contributor Author

Might have to give the NFL a pull request to clean up their readme under npm (it looks fine on GitHub). ;-)

@chadwhitacre
Copy link
Contributor Author

see about the broken img src on http://localhost:8537/on/npm/react-router/

The raw README has:

<img src="/logo/[email protected]" height="150"/>

@chadwhitacre
Copy link
Contributor Author

It's coming through unchanged in /on/npm/react-router/, so we're trying to load (via 302):

http://localhost:8537/~logo/[email protected]

@chadwhitacre
Copy link
Contributor Author

@chadwhitacre
Copy link
Contributor Author

That's (more or less) the same as what's on GitHub itself.

There was an option for this in marky-markdown, iirc ...

@chadwhitacre
Copy link
Contributor Author

serveImagesWithCDN: false, // use npm's CDN to proxy images over HTTPS

Maybe that's slightly inaccurate?

@chadwhitacre
Copy link
Contributor Author

@chadwhitacre
Copy link
Contributor Author

Looks like this is what we want to trigger.

@chadwhitacre
Copy link
Contributor Author

We need to pass an opts.package.repository.url.

@chadwhitacre
Copy link
Contributor Author

D'oh! I think that means we need to refetch or somehow reload all packages to get repository URLs out of the package.json.

@chadwhitacre
Copy link
Contributor Author

Any other plugin options we should be aware of?

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Nov 29, 2016

CDN would require version if we wanted it.

@chadwhitacre
Copy link
Contributor Author

Here is where we might find a solution to #4151 (comment).

@chadwhitacre
Copy link
Contributor Author

I'm not entirely sure how the CDN plugin relates to the GitHub plugin, but so far on my read the latter is what we want. The two do similar things, and we don't have all versions so I'm not sure storing a version is what we want. Let's go with a repo URL and the GitHub plugin since that should fix the presenting case at any rate.

@chadwhitacre
Copy link
Contributor Author

Should we throw the whole package JSON into a JSON field, or add a field for repo URL, or ... ?

@chadwhitacre
Copy link
Contributor Author

Seems like we should figure out the CDN plugin as well, to avoid having to do this over again down the line.

@chadwhitacre
Copy link
Contributor Author

When does that manifest? What is the order of plugins?

@chadwhitacre
Copy link
Contributor Author

Gosh, this is all gonna change as GitHub migrates to CommonMark.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Nov 29, 2016

This is such a giant security hole. :-/ And it doesn't scale. Let's say we trust npm ... are we going to trust all N package managers on Libraries.io? I think we're going to need to wrap this in a service that is served on a different domain and iframed in. Or we can say heck with readmes. Do people really need the readme duplicated on Gratipay? They just need to know it's the right project. A link over to npm can solve that, right? We don't have any significant content on /on/twitter/oprah/.

@chadwhitacre
Copy link
Contributor Author

Maybe some stats like downloads and/or ndependents would indicate quickly that this is the "right" project?

screen shot 2016-11-29 at 4 55 32 pm

@chadwhitacre
Copy link
Contributor Author

I'm not finding stats in the API.

@chadwhitacre chadwhitacre deleted the on-npm-foo branch November 29, 2016 22:09
@chadwhitacre
Copy link
Contributor Author

Closing. There's no sense adding inert and contentless pages.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants