Closed basicallydan closed 9 years ago
The code looks nice and simple, and easily refactorable to allow for more then two result types. I'll let you know what I think of the ui once I get a chance to see it :wink:
Cool. I might just go ahead and do it later today (it's morning now where I am in South Korea) if you're busy at the mo. Your opinion would be helpful but we can always change the UI a little bit later!
I realized that I didn't actually respond to most of your points so here goes:
a) If you are using forkable
for the text in the actual badge, then you can use whatever is better understood in the code
b) Looks great for now
c) it would be nice to at least have an option to get the badge from the CL
forkablity
into CI builds, that probably do not want to fail if it is not forkable, but on success might want to ensure that the badge is present.a) Ah so we could just write "forkable" in the code?! Hmm. Yeah I guess so. Awfully domain-specific though. If we decide to change the name to something else (not sure how that would ever be possible since it's the best name ever :stuck_out_tongue_winking_eye: ) it'd cause problems. Gonna stick with 'ok'
b) Cool
c) Very good point re: CI builds. I'll add it to the CL response. Just after I've finished watching Mr Robot. The flag library in Forkability is fine too, have a look at this line, and this line for examples. Easy to add new flags/options.
Excellent answers, thank you.
AH I ACCIDENTALLY CLOSED.
A) forkable is def the best name evah, but let's leave it it as you said C) I believe what I saw was commander :wink:. Also Mr. Robot is def more important
Any reason not to merge this?
Yes! Just wanted to put a flag in to show the badge in the command line, but then I'll merge. I'm gonna get onto that tonight :) (it's only 7:45 where I am). I've finished all of Mr Robot I could get my hands on so there's nothing else to do, really...
--Dan danhough.com http://www.danhough.com - @basicallydan http://twitter.com/basicallydan
On 26 August 2015 at 19:43, Mordechai Zuber notifications@github.com wrote:
Any reason not to merge this?
— Reply to this email directly or view it on GitHub https://github.com/basicallydan/forkability/pull/56#issuecomment-134941971 .
No pressure, I am just trying to clean up the issues list :)
I like it! Won't be long.
--Dan danhough.com http://www.danhough.com - @basicallydan http://twitter.com/basicallydan
On 26 August 2015 at 19:53, Mordechai Zuber notifications@github.com wrote:
No pressure, I am just trying to clean up the issues list :)
— Reply to this email directly or view it on GitHub https://github.com/basicallydan/forkability/pull/56#issuecomment-134946617 .
@M-Zuber So I've made it so that the badge output is very, very clearly a "failure" if it is one by making the text red and putting the word "failure". Let's put in a badge if anybody asks for it.
As well as that I've made the output a little more readable and useful. Have a look!
Sample output (from forkability)
# Forkability found 8 recommended features, and has 0 suggestions
# Features (8)
✓ Contributing document
✓ Readme document
✓ License document
✓ Changelog document
✓ .gitignore file
✓ Test suite
✓ All open issues have been acknowledged
✓ Tags are being used
---
# Suggestions (none)
---
# Forkability Badge (success)
## Just the SVG:
https://img.shields.io/badge/forkable-yes-brightgreen.svg
## Markdown:
[![This is a forkable respository](https://img.shields.io/badge/forkable-yes-brightgreen.svg)](https://basicallydan.github.io/forkability/?u=basicallydan&r=forkability)
## HTML:
<a href="https://basicallydan.github.io/forkability/?u=basicallydan&r=forkability"><img alt="This is a forkable respository" src="https://img.shields.io/badge/forkable-yes-brightgreen.svg"></a>
Pretty sure you'll be cool with this ^^
:ship: :+1: Great!! Thank you. There is still a question open for you at https://github.com/badges/shields/issues/502
Right, I've added badges support by doing the following:
badge
, which has four of its own properties:type
, which is either'ok'
or'fail'
at the momentsvg
, just a link to either the pass or fail SVGmarkdown
, a link to the Forkability page and the image in markdownhtml
, a link to the Forkability page and the image in html'ok'
. Anything below 90% is a fail, and they won't even be given a badge.textarea
s whose contents can be selected using little "(select all)" links (I understand many people dislike select-on-clicktextareas
)So what I'd like to know from anybody reading this (mainly @M-Zuber I'm guessing) is some opinions about:
a) The new addition to the callback response in the API (e.g., what do we think of "ok" instead of "pass" because "pass" sounds a little too opinionated esp. considering 90% is OK) b) The copy & layout on the page c) Whether the SVG, markdown and HTML should be included in the command line API. d) Should we try to encourage the user to pass by showing them what their badge could look like, or perhaps showing them a failure badge?
I shall eagerly await some input...!