Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Update README.md #110

Merged
merged 5 commits into from
Jan 29, 2018
Merged

Update README.md #110

merged 5 commits into from
Jan 29, 2018

Conversation

daviddias
Copy link
Member

Let's make the next release the one where the name gets updated :)

Copy link
Member

@vmx vmx left a comment

Choose a reason for hiding this comment

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

I prefer having as much Markdown (and not HTML) as possible in the README (to make it easier for automation/3rd party tools). Hence I wouldn't center the badges.

Also this doesn't follow the Standard Readme anymore. I've created a version that follows Standard Readme at https://github.com/vmx/tmp-ipld-readme

README.md Outdated

> JavaScript implementation of the IPLD Resolver (internal DAG API module)
<h1 align="center">
<a href="libp2p.io"><img width="250" src="https://ipld.io/img/ipld-logo.png" alt="libp2p hex logo" /></a>
Copy link
Member

Choose a reason for hiding this comment

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

alt text should be "ipld hex logo"

README.md Outdated

> JavaScript implementation of the IPLD Resolver (internal DAG API module)
<h1 align="center">
<a href="libp2p.io"><img width="250" src="https://ipld.io/img/ipld-logo.png" alt="libp2p hex logo" /></a>
Copy link
Member

Choose a reason for hiding this comment

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

This links to libp2p, but should probably link somewhere else.

@daviddias
Copy link
Member Author

daviddias commented Jan 24, 2018

I made it to follow:

Which I believe it makes it look better for the entry point module of each project.

That said, I defer the decision to you as you update this module and we republish to npm with the new name.

@vmx
Copy link
Member

vmx commented Jan 24, 2018

Then the Standards Readme badge needs to be removed (or the spec needs to be updated).

Oh, I didn't know that I'll be responsible :)

@vmx
Copy link
Member

vmx commented Jan 25, 2018

What do you think about following it https://github.com/ipld/ipld?

Here's how it would look like for js-ipld: https://github.com/vmx/tmp-ipld-readme/tree/0a9df3a687ef283de2305e1b7b0d50a71c785b60

I think it still looks nice, but it would also almost follow Standard Readme. Only a small tweak would be needed.

@diasdavid Even if it's my call, I'd like to know what you think.

@daviddias daviddias added the status/ready Ready to be worked label Jan 25, 2018
@daviddias
Copy link
Member Author

I like to see the badges centered, but not going to bikeshed that :)

Can we take out the (js-ipld) from the title to just be The JavaScript implementation of the IPLD?

@vmx
Copy link
Member

vmx commented Jan 25, 2018

I did a bit of research, the main resource in regards to the title is from a discussion on the "Standard Style" repo standard/standard#577. The short version is that first "Standard Style" supported the "Standard Readme", but then moved again away from it (which should be an alarm signal).

Anyway, I personally prefer having as little formatting on the README as I myself often read READMEs in a text editor and there it is quite annoying having HTML in the mix (hence not having the title and the badges centeres). But I can surely also see the point of having a README looking nice on Github.

Hence I propose this version: https://github.com/vmx/tmp-ipld-readme/tree/7d4e3ba760ac583af3a954bbbe31c6ba176ce0f1

There's two things that are not in line with the Standards Readme spec:

  1. It has a centered logo at the top before the title. Though I really think it looks nice and at one point it was according to the spec (Standardize Readme standard/standard#577 (comment)).

  2. The title doesn't contain the package name. I think there the spec went to far. The argument that you want to know that it's the package you're looking at (Top Header Naming: Should it always match the folder name? RichardLitt/standard-readme#26 (comment)) is too weak as you can just check the "install" section right below if you really want to know. In my opinion it needlessly clutters the title.

@daviddias
Copy link
Member Author

Thank you so much for digging and doing the research @vmx 👏🏽 very valid observations!

Let's go with your version for now and bring your evaluation of standard-readme to github.com/ipfs/community so that we reconsider its usage.

@vmx
Copy link
Member

vmx commented Jan 27, 2018

@diasdavid Sorry I force-pushed. I'll add the change I've overridden.

@daviddias
Copy link
Member Author

daviddias commented Jan 27, 2018

@vmx gave you publish perms to https://www.npmjs.com/package/ipld-resolver so that you can depreciate that and publish it as just ipld. Merge it away :)

@vmx vmx merged commit 5f9ecb9 into master Jan 29, 2018
@daviddias daviddias deleted the update-name branch January 29, 2018 22:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/ready Ready to be worked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants