Skip to content
This repository has been archived by the owner on Jan 16, 2023. It is now read-only.

Adds a fancy new README file #22

Merged
merged 1 commit into from
Nov 13, 2017
Merged

Adds a fancy new README file #22

merged 1 commit into from
Nov 13, 2017

Conversation

TzviPM
Copy link
Contributor

@TzviPM TzviPM commented Jul 27, 2017

After discussions with the team, I decided I would try my hand at a "nice" README file that may be good to turn into a template in the future.

Inspiration

Using an HTML header

I decided to go with an HTML header, because I noticed that the README files I liked in the awesome-readme repo all did this.

Centred

I also liked the centred design better than a left-aligned version.

Note this is officially a markdown anti-pattern (as is any formatting); however, I don't think that's a big deal for GFM, since it's not "official" markdown. Also, seeing as how many READMEs are using this pattern (at least for the header), I don't see a problem with it.

@TzviPM TzviPM self-assigned this Jul 27, 2017
README.md Outdated
</a>
</p>

<div align="center">
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit picking and opinionated a bit. Feel free to argue against this.

I prefer this:

<div align="center">
    <a href="https://shopify.com/">Website</a>
    <span> | </span>
    <a href="https://polaris.shopify.com/">Styleguide</a>
    <span> | </span>
    <a href="http://github.com/Shopify/javascript-utilities/blob/master/CHANGELOG.md">Changelog</a>
</div>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm indifferent, tbh, although I should become opinionated.

My thoughts with the current styling are that it helps, in general, to spot and fix typos when the text is on its own line.

Copy link
Contributor

@ismail-syed ismail-syed left a comment

Choose a reason for hiding this comment

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

This is looking pretty 👌

A tad overkill for this repo but if others are okay with this style of README.md let's add it to the @shopify/javascript .

It helps makes our OSS presence a bit more polished.

README.md Outdated
A set of utilities for JavaScript and TypeScript projects at Shopify.
<h1 align="center">
<br>
<a href="http://www.shopify.com/"><img src="https://cdn.shopify.com/assets2/press/brand/shopify-logo-main-small-f029fcaf14649a054509f6790ce2ce94d1f1c037b4015b4f106c5a67ab033f5b.png" alt="Shopify" width="200" /></a>
Copy link
Member

Choose a reason for hiding this comment

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

Really don’t like the big logo at the top, if we want to include it I think it should be below the license.

README.md Outdated
<br>
</h1>

<h4 align="center">A set of utilities for JavaScript and TypeScript projects at Shopify.</h4>
Copy link
Member

Choose a reason for hiding this comment

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

We can’t just toss a doc with styles to center around this whole thing?

README.md Outdated

<div align="center">
<a href="https://shopify.com/">
Website
Copy link
Member

Choose a reason for hiding this comment

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

Why are we linking to these things? This feels so counter to what I expect from an OSS project, we shouldn’t be hiding the “what is this and why is it good and how do I use it” behind a wall of branding IMO

README.md Outdated
Styleguide
</a>
<span> | </span>
<a href="http://github.com/Shopify/javascript-utilities/blob/master/CHANGELOG.md">
Copy link
Member

Choose a reason for hiding this comment

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

This link is good but make it part of the TOC

@TzviPM
Copy link
Contributor Author

TzviPM commented Aug 9, 2017

@lemonmade, when you get back, can you take another look at this?

@TzviPM
Copy link
Contributor Author

TzviPM commented Nov 13, 2017

I'm going to go ahead and merge this in, since all comments have been addressed.

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.

None yet

4 participants