Perl 6 - the future is here, just unevenly distributed

IRC log for #pr-challenge, 2016-08-19

| Channels | #pr-challenge index | Today | | Search | Google Search | Plain-Text | summary

All times shown according to UTC.

Time Nick Message
04:12 rvandam joined #pr-challenge
06:44 amalia joined #pr-challenge
07:17 Tordek left #pr-challenge
08:45 veryrusty joined #pr-challenge
09:03 veryrusty joined #pr-challenge
09:50 veryrusty joined #pr-challenge
10:08 veryrusty joined #pr-challenge
11:57 veryrusty joined #pr-challenge
13:02 veryrusty joined #pr-challenge
17:39 jacoby Looking at a module that wasn't assigned, and I have a question of manners.
17:39 jacoby Or protocol.
17:39 genio sure
17:40 jacoby Anyway, module wraps a web API. Module includes $x->avatar()
17:40 jacoby $x->avatar() and most everything else wraps talk_to_api(), which runs everything through json->decode()
17:40 jacoby Except API returns a PNG, not JSON.
17:41 jacoby And I'm wondering what the "Right Thing" is.
17:41 jacoby ATM, I could cpan force install $MODULE and be sure that $x->avatar is a thing to not use, but that's just me.
17:42 jacoby So, pull request possibility 1 is to pull $x->avatar functionality from the module and testing.
17:43 jacoby pull request possibility 2 is to have an 'if avatar' addition to the module and write tests using maybe Perl::Magick? to see if the PNG is the expected size.
17:43 genio ah, so it seems an assumption was made that the results would always be JSON but in this case you get a raw PNG returned.  I would patch this by having ->avatar() not use the wrapper function, or by altering the wrapper function for expected return type.  hard to say without seeing the code
17:44 jacoby https://github.com/damog/www-tumblr
17:45 jacoby Specifically around line 36 of this: https://github.com/damog/www-tumblr/blob/master/lib/WWW/Tumblr/API.pm
17:46 jacoby Since I'm new to this one -- kinda had a half-considered thought about collecting ideas and looked in CPAN -- I don't know if the API change predated the module or what.
17:47 genio if ($response->is_success) { return decode_json($response->decoded_content)->{response}; } elsif ($response->code == 301 && $method_name eq 'avatar') { return $response->decoded_content; } else { ...
17:47 genio would be my first attempt.  run the testing again after that. if all's good, submit a PR
17:47 genio if I'm woefully misunderstanding the logic, rethink a bit
17:48 genio or really, if it's a 301, won't the content be a URL to the proper place? either way, the tests should point out the proper path I would think
17:48 jacoby the only failing test https://github.com/damog/www-tumblr/blob/master/t/02-blog_avatar.t
17:49 genio dump out the actual response in the test and run it a few times to see what your actual results are.
17:50 genio otherwise, maybe moving that test to xt/author/02-blog_avatar.t makes more sense than being in t/
17:50 genio These are my half-baked ideas without having time to go in-depth in the module though.
17:51 jacoby I'm half-baking it here, too.
17:53 jacoby Looks like the API has no mechanism for changing avatar, only getting it.
17:53 jacoby https://github.com/damog/www-tumblr/blob/master/lib/WWW/Tumblr/Blog.pm
17:54 jacoby If I decide that 'broken module is broken' and do a PR to remove it, I know how to do that, but that has problems.
17:55 jacoby If I decide that 'everything else expects JSON, so I can make an object to spit back the URL, I can do that.
17:56 jacoby If I decide 'simplest thing is to spit out a PNG', I can do that.
17:56 jacoby But what is the right thing?
18:01 jacoby Looking at the test, looks like generating and spitting out the URL might be the right thing. Making the code follow the test is right, right?
18:03 jacoby Or, make three pull requests and make it damog's problem.
18:16 genio I would follow the test.  Before opening a PR, I would check other language's APIs for the similar method call to see what it is that they do.
19:42 dod joined #pr-challenge
19:47 dod joined #pr-challenge
22:09 veryrusty joined #pr-challenge

| Channels | #pr-challenge index | Today | | Search | Google Search | Plain-Text | summary