Maciej Szulik added the comment: > I have updated the patch.
Thank you, the patch itself LGTM. > > We do need proper error handling, still. > The problem is "err" variable has stdout of that process. So, even if we > change/push branch, it will be non-empty. Any idea how to solve this problem? If I'm reading this [1] correctly communicate returns you a tuple where first argument is stdout contents and second stderr, isn't that what you want? > One more thing, if we create PR using API, it triggers GitHub webhooks. So, > I'm removing the part where we are storing the response in DB. Yup, your webhook should handle that part nicely, already. > Also, I'm not sure how to add tests for this. Yeah, let's leave it like this for now. [1] https://docs.python.org/2/library/subprocess.html#subprocess.Popen.communicate _______________________________________________________ PSF Meta Tracker <metatrac...@psf.upfronthosting.co.za> <http://psf.upfronthosting.co.za/roundup/meta/issue600> _______________________________________________________ _______________________________________________ Tracker-discuss mailing list Tracker-discuss@python.org https://mail.python.org/mailman/listinfo/tracker-discuss Code of Conduct: https://www.python.org/psf/codeofconduct/
