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

Add screenshots for all GitHub readme examples #41

Closed
wants to merge 3 commits into from
Closed

Add screenshots for all GitHub readme examples #41

wants to merge 3 commits into from

Conversation

deltaidea
Copy link

@deltaidea deltaidea commented May 25, 2017

... except for b4b4r07/dotfiles, that project has practically no readme at the time of this PR. Should probably remove it from the list.

All screenshots are 980 x 1064 == (width of GiHub layout) x (1080 display - minimal cropped header).

Please take a look at the HTML. It is good enough? I couldn't make it less verbose or translate to Markdown.
Please feel free to ignore this PR if this is not acceptable - it didn't make that much time since I managed to automate almost every step. :)

Edit: Live preview: https://github.com/deltaidea/awesome-readme/tree/screenshots

Closes #1.

@RichardLitt
Copy link
Contributor

I suppose there is no way to embed the versioned screenshot of the README? Right now it links to the repository, which may not reflect the screenshot.

Let's merge the other PRs, first, and then rebase this. I'm not sure I want this, as it makes the markdown inscrutable - will depend on @matiassingers thoughts.

@deltaidea
Copy link
Author

I suppose there is no way to embed the versioned screenshot of the README? Right now it links to the repository, which may not reflect the screenshot.

I'm not sure that you mean by that. It's possible to link to the target readme at commit at which the screenshot was made - just change the repo URL to include the current commit. Want me to do that?

@RichardLitt
Copy link
Contributor

It's possible to link to the target readme at commit at which the screenshot was made - just change the repo URL to include the current commit. Want me to do that?

Exactly. That way, we won't get the issue like we had with dotfiles.

I've gone through and merged the other PRs. Mind redoing the PR? :)

@deltaidea
Copy link
Author

Sure! Here, I've rebased the PR. Let me come up with a clever way to fetch a list of current commits for all repos.

@deltaidea
Copy link
Author

Documented everything in contributing.md. Please review that too, as I'm not a native speaker and not completely sure how it sounds.

Copy link
Contributor

@RichardLitt RichardLitt left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. One small nitpick.

contributing.md Outdated
@@ -6,9 +6,13 @@ Please ensure your pull request adheres to the following guidelines:
- Suggested READMEs should be beautiful or stand out in some way.
- Make an individual pull request for each suggestion.
- New categories, or improvements to the existing categorization are welcome.
- Keep descriptions short and simple, but descriptive.
- Start the description with a capital and end with a full stop/period.
- When adding a new README, make sure to follow existing visual and code style:
Copy link
Contributor

Choose a reason for hiding this comment

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

Follow the existing visual...

@deltaidea
Copy link
Author

Thanks, fixed that.

@deltaidea
Copy link
Author

This was a terrible idea because of the markup bloat.

@deltaidea deltaidea closed this Jun 21, 2017
@RichardLitt
Copy link
Contributor

Well, yes and no. The implementation is very bloated.

I think one thing we could do, rather easily, is just have a link to a screenshot, and not worry about the markdown at all. Just have a [Screenshot](link) link at the end of every entry. What do you think?

@deltaidea
Copy link
Author

deltaidea commented Jun 23, 2017

Yeah, I agree.
Okay, I'll add simple links. Admittedly, they aren't nearly as usable. Gotta start somewhere though.

I could also automate taking the screenshots themselves. Would you accept something like npm run screenshots? I'd make a script that opens each repo in PhantomJS and updates the screenshot if the latest commit isn't the same as on the last run.

@RichardLitt
Copy link
Contributor

That would work for me. The only issue is that we need to make sure that the screenshots match the commit of the PR, not the most recent commit in whatever given README.

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.

2 participants