These are chat archives for cherrypy/cherrypy

18th
Feb 2017
Krystian Rosiński
@krysros
Feb 18 2017 12:09
Hi. I just installed latest version of CherryPy on Windows 10 and run (as an example) built-in "hello world" tutorial. When I pressed Ctrl+C to shutdown an app (line 32), I got the following exception gist.
Is this normal behavior or a bug?
Jason R. Coombs
@jaraco
Feb 18 2017 14:35
Krystian: Looks like a bug to me.
@krysros: ^
Krystian Rosiński
@krysros
Feb 18 2017 14:59
@jaraco Ok. I have found an cherrypy/cherrypy#1342 opened on 3 Oct 2014 - left without comment so far.
Jason R. Coombs
@jaraco
Feb 18 2017 15:47
@krysros Thanks for linking it. Yes, looks to be the same issue. Also looks like @cyraxjoe laid out a viable solution. Perhaps you or he could work up a PR to fix the issue.
Sviatoslav Sydorenko
@webknjaz
Feb 18 2017 19:26
the PR is almost ready for merge: cherrypy/cherrypy#1564
@jaraco @cyraxjoe you'll probably want to take a look
Jason R. Coombs
@jaraco
Feb 18 2017 22:05
So a couple of things I notice. First, I notice that comments like this (https://github.com/cherrypy/cherrypy/pull/1564#discussion_r101898692) are asking for work outside the scope of the request… It’s not appropriate to ask new contributors to fix unrelated issues just because you noticed them. In fact, if the contributor had made these changes as part of the inital request, I would have asked them to be made separately or at least as separate commits to avoid conflating separate issues.
Second, I would have requested an update to the changelog. Without an update to the changelog, this change is incomplete. The entry in the changelog should reference the original issue that this PR fixes. The PR itself should also reference the original issue.
That way, it will be possible to trace the change to the release.
Third, I wouldn’t have done a squash merge. Here’s why: https://blog.jaraco.com/dont-use-squash-and-merge/
Oh, I do see for the second item that the original issue was referenced in the PR. Good work on that.
(both in creating the template and in supplying the issue)
Krystian Rosiński
@krysros
Feb 18 2017 22:33
@jaraco @webknjaz @cyraxjoe Thanks