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

modernize our README #2232

Merged
merged 8 commits into from Jan 7, 2020
Merged

modernize our README #2232

merged 8 commits into from Jan 7, 2020

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Jan 6, 2020

This mainly removes a lot of overhead in favor of our dedicated docs page and preact-awesome list

@developit do you have the link to your gist with the size trial? I'd like to include it here as well to point to if needed

TODO: compare preact-awesome with removed links

@coveralls
Copy link

coveralls commented Jan 6, 2020

Coverage Status

Coverage increased (+0.07%) to 97.749% when pulling a47c85b on update-readme into 398eb99 on master.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
ooade pushed a commit to preactjs/awesome-preact that referenced this pull request Jan 6, 2020
In the preact repository we are removing a lot of content from our README, I'm adding it here for visibility (+ linking to here) preactjs/preact#2232
return (
<div>
<p>Do you agree to the statement: "Preact is awesome"?</p>
<input value={input} onChange={e => setInput(e.target.value)} />
Copy link
Member

Choose a reason for hiding this comment

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

I worry about using Hooks for the first example, since this sets folks up to end up with stale closures and event handler thrashing without any context. Maybe we should do a side-by-side of hooks and classes to demonstrate?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well I think it's pretty linear to just exposing them to doing this in a Component, no? What do you have in mind?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the community is moving toward Hooks right now, I think making the first example Hooks driven gives additional confidence the author written code will not need to be significantly change or refactored.

Copy link
Member

Choose a reason for hiding this comment

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

+1 For a hooks driven example. My gut tells me that users coming from React will perceive Preact as "old" when the first example only shows classes.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
JoviDeCroock and others added 2 commits January 6, 2020 16:30
Co-Authored-By: Jason Miller <developit@users.noreply.github.com>
Co-Authored-By: Jason Miller <developit@users.noreply.github.com>
Copy link
Collaborator

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Left a few wording comments, no need to apply but thought some additional changes might benefit readers.

Themes:

  • Reduce the usage of 'you', 'we', 'us'.
  • Keep the separation of 'calculated differential' and 'application' more severe.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
return (
<div>
<p>Do you agree to the statement: "Preact is awesome"?</p>
<input value={input} onChange={e => setInput(e.target.value)} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the community is moving toward Hooks right now, I think making the first example Hooks driven gives additional confidence the author written code will not need to be significantly change or refactored.

Copy link
Collaborator

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Looks great to me, let's get a few other's input to be sure though.

@@ -9,7 +9,7 @@

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
<p align="center">Fast <b>3kB</b> alternative to React with a similar API.</p>

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for the line above just a proposition

@JoviDeCroock JoviDeCroock merged commit 9566966 into master Jan 7, 2020
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.

None yet

7 participants