Add merged attribute to pulls, fix #335

[ekmartin]

Also changes is_merged to return .merged if it's True. If it's False or None it still does an API request.

[sigmavirus24]

if it is not None. If it is False or True it came back from the API. You may not want to do this at all though because the cached information can be out of date.

[ekmartin]

I was thinking that if a pull is merged, self.merged will always equal True in the future (even reverting the pull doesn't seem to change that), so there wouldn't be a point in checking the API. However, if self.merged is False, you'd probably want to be able to check the API to see if it has been merged yet?

The name of the method (is_merged) is a bit problematic though, as it sounds more like a getter for the merged attribute than something that would actually fire of an API request.

[sigmavirus24]

So the name is confusing if this is the first part of github3.py with which you interact with. Everything else does not retrieve a specific attribute and the merged attribute is a new addition to what the API returns. (Hence why it was missing and is_merged was not a bad name choice at the time.)

The pattern with the library is that attributes do not retrieve data for you or make API calls, but methods do. It's a logical distinction.

I would say someone can rely on pr.merged existing but they need to check that it's actually a boolean instead of just using is pr.merged. That said, I'm still conflicted about short-circuiting the method. I can't think of a place we do that elsewhere but I'm also in favor of saving people a request against their ratelimit.

[ekmartin]

Yep, I understand. I don't mind removing the short-circuiting if you feel that's a better solution, just thought it would work alright here since as far as I know a merged pull request can never change status.

[sigmavirus24]

I'm not too concerned about it.

Thanks for the PR @ekmartin!