architv / soccer-cli

:soccer: Football scores for hackers. :computer: A command line interface for all the football scores.
MIT License
1.09k stars 221 forks source link

Converts UTC time string to the local user time in console #65

Closed carlosvargas closed 8 years ago

carlosvargas commented 8 years ago

For live matches, the time for the match is in UTC, which makes sense for an API. However when printing it out to the console, UTC doesn't make much sense since you have to force the user to covert it to their local time.

So instead of printing: =================== UEFA Champions League ==================== Arsenal - vs - Barcelona 7:45 PM UTC Juventus - vs - Bayern Munich 7:45 PM UTC

It's now localized to the user (in this case, me :smile: ): =================== UEFA Champions League ==================== Arsenal - vs - Barcelona 11:45 AM Juventus - vs - Bayern Munich 11:45 AM

Saturn commented 8 years ago

Could the convert_utc_to_local_time function be a staticmethod instead?

carlosvargas commented 8 years ago

Yeah, that makes sense. I'll update the PR when I get a chance.

carlosvargas commented 8 years ago

@Saturn should I keep it a function of Stdout or move it up to the parent BaseWriter, just in case the other writers want to use it? Or just move it into a module method?

Saturn commented 8 years ago

Probably keep it stdout for now at least.

carlosvargas commented 8 years ago

Updated PR.

Saturn commented 8 years ago

It works fine having tested it.

Slightly related. Does anyone else really prefer the time written in 24hr format instead of am/pm. In my experience the kick off and starting times of events are nearly always given in 24 hour format. Is it different in other countries?

carlosvargas commented 8 years ago

So it appears that most people are on 24 hour format

map

ueg1990 commented 8 years ago

i think for now we can keep it 24 hours and in a separate PR add a command line option to allow users to choose between am/pm or 24 hour format

Saturn commented 8 years ago

Interesting and useful graphic there.

I think it's only the data from the live site that outputs the 12 hour style. Maybe that is just @architv 's preference.

My preference is to default to a 24 hour clock and then potentially offer the option for people who prefer the 12 hour style instead.

It is a really minor issue though.

carlosvargas commented 8 years ago

I went ahead and made it display the time using 24 hour and added the flag to use 12 hour format if desired.

architv commented 8 years ago

@carlosvargas The updated works perfectly fine. It makes a lot of sense to implement 24 hour clock format for timings. Thanks!