hstove / issue_stats

Analyze and compare how long it takes for Github issues to be closed.
issuestats.com
MIT License
184 stars 13 forks source link

Fix #23 decrease horizontal width of badges #26

Closed brettwooldridge closed 9 years ago

brettwooldridge commented 9 years ago

This pull request provides greater conciseness of badge text, as per the #23 enhancement request. Several formats were proposed there, one was chosen for implementation.

All permutations of Issue badge text in the pull request:

[issue closure | < 1 min] [issue closure | 23 min] [issue closure | ~1 hr] [issue closure | 9 hrs] [issue closure | 1 day] [issue closure | 23 days] [issue closure | ~1 mon] [issue closure | 10 mon] [issue closure | > 2 yrs]

The two entries with the ~ sign map to the 'about' in the previous badge, but I actually question the utility of it -- their presence in the original (current) badges being a side-effect of using the distance_of_time_in_words() Ruby function.

I would actually prefer to consolidate these a bit, and resubmit this pull, if you agree. Here is what I think is simpler, yet equally meaningful to users:

[issue closure | X min] [issue closure | X hrs] [issue closure | X days] [issue closure | X mon] [issue closure | X yrs]

The variant for hours, days, and years would account for singular and plural forms (eg. 1 hr, 2 hrs).

hstove commented 9 years ago

I really like having the option of a more concise badge, but I'd only merge this if it was only shortened as an option passed as a parameter to the badge URLS, like ?concise=true. This Param would get passed through to the badge_url method and then your logic would be applied only if the parameter was found.

brettwooldridge commented 9 years ago

Great! I'll take a look, and update the pull request when its done.

brettwooldridge commented 9 years ago

Updated with support for ?concise=true or more tersely ?concise. Default behavior is as before.

hstove commented 9 years ago

This looks great, that's exactly what I was thinking. The (probably) final thing I'll ask is to add unit tests for 'badge_url' with the concise option. Would you mind adding a test or two to https://github.com/hstove/issue_stats/blob/master/spec/models/report_spec.rb ?

brettwooldridge commented 9 years ago

Believe it or not this is my first experience with Ruby, but I'll take a look at the existing tests to figure it out.

hstove commented 9 years ago

Don't worry about it, I'm doing a minor refactor and adding some tests so I'll take care of it. Thanks a bunch for the code! Nice job for a first time Rubyer.

hstove commented 9 years ago

Thanks!

brettwooldridge commented 9 years ago

No, thank you for the nice project.