Skip to content
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

Update README.md header #3251

Merged
merged 4 commits into from
Aug 13, 2019

Conversation

davidedistefano
Copy link
Contributor

This PR updates the README.md layout visually, adding the full Solidus logo, improving the header and the Open Collective section.

Description
This PR could be the first of many improvements to this project README.md, like for example adding admin screenshots or a good project description.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

The GitHub preview is not the only way of viewing this file. I am 👎 for this change.

README.md Outdated

<p align="center">
<strong>A free, open-source ecommerce platform that gives you complete control over your store.</strong>
</p>
Copy link
Member

Choose a reason for hiding this comment

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

I am not a fan of using more and more html in our readme file. First it is a markdown file and secondary this file is also meant to be read in an editor or terminal. Please do not see the GitHub preview as the single way to view this file.

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

I think this looks really good in GitHub and I think it makes it a lot easier to find relevant information.

I do share Thomas' concern, however, that this file is not exclusively seen in GitHub.

@davidedistefano
Copy link
Contributor Author

davidedistefano commented Jul 4, 2019

@tvdeyen @jacobherrington I understand your concern, but I think it's a matter of choice which kind of users to please when we talk about a README file, whether editor/terminal users or GitHub users. I know the first choice is more dev-friendly, but a lot of people use GitHub, and in my opinion, a good-looking README is good for the success of the platform. In fact, many successful projects, such as Vue.js, have a hybrid (HTML/markdown) file.

By the way, this is not a crucial change for now, and we can find a compromise, but I think, in the future, we should have more flexibility in terms of how to update the README, such as adding new content like images (which are not displayed in the editor/terminal) and sponsors logos.

@tvdeyen
Copy link
Member

tvdeyen commented Jul 4, 2019

@davidedistefano the GitHub readme is not a marketing page. We have a nice looking site for this.

a good-looking README is good for the success of the platform

A good looking readme is possible with plain markdown. Centering text is personal taste and does neither decide over the success of this plattform nor lead to a better looking readme.

@kennyadsl
Copy link
Member

IMHO, the README is part of the marketing material, especially if the target of the thing to market is mostly made by developers. We can argue that some developers use the README in the editor and not via GitHub, but honestly, I only have my intuition here, no real data.

Anyway, I think Thomas represents a part of users that prefer not to see HTML code in the README and we should listen to this feedback.

I suppose that centering text is something that we can easily change, probably a nice-looking table with partners images is more concerning, right @davidedistefano?

@davidedistefano
Copy link
Contributor Author

I removed the HTML tags where possible in favor of markdown.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks, @davidedistefano!

Copy link
Contributor

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

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

LGTM, I agree with @tvdeyen and would prefer little to not HTML in the README but this looks fine to me.

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

I like this as well, but we shouldn't merge it without @tvdeyen weighing in on it at its current state.

WDYT Thomas?

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

I am pretty sure we can get rid of HTML in favor of markdown for the linked images

README.md Outdated
Thank you to all our donors! 🙏 [Become a donor](https://opencollective.com/solidus#backer)
<a href="https://nebulab.it/" target="_blank">
<img width="240px" src="https://nebulab.it/assets/images/public/logo.svg">
</a>
Copy link
Member

Choose a reason for hiding this comment

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

Like with the status badges should also be writable as:

[![Nebulab](https://nebulab.it/assets/images/public/logo.svg)](https://nebulab.it/)

README.md Outdated
<a href="https://www.enginecommerce.com/"><img src="https://images.opencollective.com/proxy/images?src=https%3A%2F%2Flogo.clearbit.com%2Fenginecommerce.com&height=100"></a>
<a href="https://supergood.software/"><img src="https://images.opencollective.com/proxy/images?src=https%3A%2F%2Fopencollective-production.s3-us-west-1.amazonaws.com%2F3bbb1440-727f-11e9-a366-37673cc38cee.png&height=100"></a>
<a href="https://karmacreative.io/"><img src="https://images.opencollective.com/proxy/images?src=https%3A%2F%2Fopencollective-production.s3-us-west-1.amazonaws.com%2Fab94d2a0-7253-11e9-a366-37673cc38cee.png&height=100"></a>
|<a href="https://www.enginecommerce.com/" target="_blank"><img src="https://images.opencollective.com/proxy/images?src=https%3A%2F%2Flogo.clearbit.com%2Fenginecommerce.com&height=100"></a>|<a href="https://supergood.software/" target="_blank"><img src="https://images.opencollective.com/proxy/images?src=https%3A%2F%2Fopencollective-production.s3-us-west-1.amazonaws.com%2F3bbb1440-727f-11e9-a366-37673cc38cee.png&height=100"></a>|<a href="https://karmacreative.io/" target="_blank"><img src="https://images.opencollective.com/proxy/images?src=https%3A%2F%2Fopencollective-production.s3-us-west-1.amazonaws.com%2Fab94d2a0-7253-11e9-a366-37673cc38cee.png&height=100"></a>|
Copy link
Member

Choose a reason for hiding this comment

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

These should also be writable as markdown. HTML should not be necessary

@kennyadsl
Copy link
Member

@tvdeyen we can reduce HTML further in favor or Markdown but some images need to be resized (they are SVG) and it cannot be done with GitHub markdown, unfortunately. Do you have any suggestion?

@tvdeyen
Copy link
Member

tvdeyen commented Aug 9, 2019

@kennyadsl the Nebulab logo could have a viewport in the svg with the correct size and therefore will not need a size attibute on the img tag. See http://tutorials.jenkov.com/svg/svg-viewport-view-box.html

@davidedistefano
Copy link
Contributor Author

Now everything is written in Markdown except for the Solidus logo, which was already written in HTML.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks for being patient with me. 👍

@davidedistefano davidedistefano force-pushed the davidedistefano/update-readme branch from 7c8ea0b to 56deee8 Compare August 13, 2019 09:40
This update improves the README.md layout visually, adding the full Solidus logo, improving the header and the Open Collective section.
@davidedistefano davidedistefano force-pushed the davidedistefano/update-readme branch from 56deee8 to fa86a23 Compare August 13, 2019 09:47
@kennyadsl kennyadsl merged commit d2c71a6 into solidusio:master Aug 13, 2019
@kennyadsl kennyadsl deleted the davidedistefano/update-readme branch August 13, 2019 10:04
jacobherrington added a commit to jacobherrington/solidus that referenced this pull request Aug 14, 2019
…-readme

Update README.md header

Co-authored-by: jacobherrington <jacobherringtondeveloper@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants