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

update the script tag in an example provided by README.md #421

Merged
merged 2 commits into from
Aug 18, 2016
Merged

update the script tag in an example provided by README.md #421

merged 2 commits into from
Aug 18, 2016

Conversation

Mithgol
Copy link
Contributor

@Mithgol Mithgol commented Aug 16, 2016

Fixes #418.

@jbenet jbenet added the status/deferred Conscious decision to pause or backlog label Aug 16, 2016
```html
<script type="text/javascript" src="assets/bundled.js"></script>
```
The last published version of the package becomes available on [npmcdn](https://npmcdn.com/) and thus you may use it as the source:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can link to the latest npmcdn dist version with this I believe [npmcdn](https://npmcdn.com/ipfs@*/dist/)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The homepage of https://npmcdn.com/ currently says the following:

You may also use a tag or version range instead of a fixed version number, or omit the version/tag entirely to use the latest tag.

And that's why I've decided to omit the version/tag.

Copy link
Member

@nginnever nginnever Aug 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh cool, omitting the version tag entirely is an easy way to link to the latest dist

https://npmcdn.com/ipfs/dist/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm: do you suggest to link the word “npmcdn” to https://npmcdn.com/ipfs/dist/ instead of the npmcdn's homepage?

Originally I intended to use the examples (<script src="https://npmcdn.com/ipfs/dist/index.min.js"> and <script src="https://npmcdn.com/ipfs/dist/index.js"> below) to explain the script's location but I left the word “npmcdn” to tell the ignorant public what npmcdn is. That's how the README.md of ipfs-api does it currently, for example.

Copy link
Member

@nginnever nginnever Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah exactly, I was just suggesting the link could be directed towards the latest dist instead of the cdn homepage since sometimes I want to download a dist for requiring in my own project structures without relying on npm to be online.

EDIT

A better link would be to a content addressable hash of the dist that was backed by a financially incentivized swarm to keep it alive :)

Copy link
Contributor Author

@Mithgol Mithgol Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still compelled to keep the hyperlink to https://npmcdn.com/ipfs/dist/ because it's educational and, in fact, that's how I've learnt of npmcdn myself (from a README in another repo).

That's why I've just added yet another hyperlink that leads to https://npmcdn.com/ipfs/dist/ and is titled “available for download” for people ready to download the dist manually.

@daviddias
Copy link
Member

Other than @nginnever comment LGTM :)

@daviddias daviddias merged commit 0cb3490 into ipfs:master Aug 18, 2016
@daviddias daviddias removed the status/deferred Conscious decision to pause or backlog label Aug 18, 2016
@daviddias
Copy link
Member

thank you :)

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.

4 participants