Closed dobbscr closed 11 years ago
Hi @dobbscr,
Thanks for your interest and i'm glad you've found the module useful. As you can see i've made a number of comments on your patch, mainly around keeping it in line with the existing API. I note that in a subsequent change you've already removed some commented print statements i mentioned.
Also, there are no test cases provided for this new functionality. I much prefer patches that extend the existing test suite to cover new functionality and bugfixes. I will accept a patch missing this but please look at it if you have time.
If you could please make the changes and send a pull request with one commit to cover the changes i've noted I'd look forward to merging in these features. You can include any test cases in the same commit or separately, as you wish.
Thanks again for your efforts!
@aelse :+1: yes, new functionality is always welcome, as long it is has the same or better test-coverage than the existing codebase. That's a good practice.
@dobbscr Please let me know if you're willing and able to look at my suggestions for improving the patch. If you're not, that's fine and I'll close the request. Thanks!
Yep…looks good!!
Fra: Alexander Else [mailto:notifications@github.com] Sendt: 12. august 2013 06:21 Til: aelse/python-crowd Kopi: Christopher Dobbs Emne: {Disarmed} Re: [python-crowd] New functions added. (#18)
@dobbscrhttps://github.com/dobbscr Please let me know if you're willing and able to look at my suggestions for improving the patch. If you're not, that's fine and I'll close the request. Thanks!
— Reply to this email directly or view it on GitHubhttps://github.com/aelse/python-crowd/pull/18#issuecomment-22472739.
Will look at implementing your comments as soon as i get back from vacation... Cheers Chris
Hi Aelse, I liked your crowd rest api module so much I decided to extend it with some extra functions I needed.