)]}'
{"keystone/tests/unit/test_hacking_checks.py":[{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"1090d9f54125bcf72019e485de54832859fada44","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        lines \u003d textwrap.dedent(code).strip().splitlines(True)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        guide \u003d pep8.StyleGuide(select\u003d\u0027K333\u0027)"},{"line_number":43,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines, options\u003dguide.options)"},{"line_number":44,"context_line":"        checker.check_all()"},{"line_number":45,"context_line":"        checker.report._deferred_print.sort()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_5fa23b9a","line":42,"range":{"start_line":42,"start_character":40,"end_line":42,"end_character":44},"updated":"2017-01-05 13:12:22.000000000","message":"what is this? It is better having a constant/variable to name it.","commit_id":"bae3dce8ab58e6bfd716fb59f8f3accecf2acf35"},{"author":{"_account_id":17860,"name":"Samuel de Medeiros Queiroz","email":"samueldmq@gmail.com","username":"samueldmq"},"change_message_id":"0d6e5964de68db906cf51bd11ea7e08a8770b7f5","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        lines \u003d textwrap.dedent(code).strip().splitlines(True)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        guide \u003d pep8.StyleGuide(select\u003d\u0027K333\u0027)"},{"line_number":43,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines, options\u003dguide.options)"},{"line_number":44,"context_line":"        checker.check_all()"},{"line_number":45,"context_line":"        checker.report._deferred_print.sort()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_312d9c95","line":42,"in_reply_to":"ba5201f7_0e1a397d","updated":"2017-01-05 18:53:11.000000000","message":"I think it a comment could be enough.\n\nSame comment as in https://review.openstack.org/#/c/416829/\n\nIt\u0027s not clear why this replaces the mocking above. The old comment says the mock was needed to reset all the checks and only make available the registered ones in L28","commit_id":"bae3dce8ab58e6bfd716fb59f8f3accecf2acf35"},{"author":{"_account_id":24681,"name":"Adam Williamson","email":"awilliam@redhat.com"},"change_message_id":"aedd2814c3cb08de1e161b1384a28e09986d9c64","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        lines \u003d textwrap.dedent(code).strip().splitlines(True)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        guide \u003d pep8.StyleGuide(select\u003d\u0027K333\u0027)"},{"line_number":43,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines, options\u003dguide.options)"},{"line_number":44,"context_line":"        checker.check_all()"},{"line_number":45,"context_line":"        checker.report._deferred_print.sort()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_51b22007","line":42,"in_reply_to":"ba5201f7_312d9c95","updated":"2017-01-05 19:08:50.000000000","message":"So I wrote this change as a patch to python-keystoneclient , which isn\u0027t really the same at all. I don\u0027t know what\u0027s going on with how Steve has adapted my patch.\n\nYou can see my original patch here:\n\nhttps://bugs.launchpad.net/python-keystoneclient/+bug/1652458/+attachment/4796141/+files/0001-Work-with-pep8-and-pycodestyle-don-t-use-pep8-intern.patch\n\nIt\u0027s changing a *specific* test class - TestCheckOsloNamespaceImports - a test that\u0027s particular to this K333 check. I don\u0027t know what the relation is between that file and this one, but the change indeed doesn\u0027t look like it makes much sense when applied to a more *generic* class (which looks like it\u0027s intended to be inherited by various different test classes for different pep8 checks) like this.\n\nWhat the change actually does is construct a pep8 / pycodestyle StyleGuide instance which will only run checks for a test called \u0027K333\u0027 - if you specify the \u0027select\u0027 arg for a StyleGuide instance, it will *only* select checks that match the value - and feed that StyleGuide to the checker, so the checker will only run that check.\n\nI\u0027m not terribly familiar with this code, but just eyeballing it, to make sense in this context, the patch should be adjusted to get the value for the \u0027select\u0027 arg from the checker instance returned by get_checker (rather than hardcoding \u0027K333\u0027).","commit_id":"bae3dce8ab58e6bfd716fb59f8f3accecf2acf35"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"c81e9bddd6898a353d4bf3d1dcc9fc26b43578f9","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        lines \u003d textwrap.dedent(code).strip().splitlines(True)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        guide \u003d pep8.StyleGuide(select\u003d\u0027K333\u0027)"},{"line_number":43,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines, options\u003dguide.options)"},{"line_number":44,"context_line":"        checker.check_all()"},{"line_number":45,"context_line":"        checker.report._deferred_print.sort()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_7d6ff52a","line":42,"in_reply_to":"ba5201f7_316afcee","updated":"2017-01-05 21:22:53.000000000","message":"\"I still don\u0027t understand why Steve took my patch for python-keystoneclient and made it into a patch for keystone, but hey.\"\n\nMy bad, I was in a rush to propose patches before logging off for the night. I remembered that we had the same K333 check in keystone (it has since been removed) and blindly replaced where the mock was.","commit_id":"bae3dce8ab58e6bfd716fb59f8f3accecf2acf35"},{"author":{"_account_id":24681,"name":"Adam Williamson","email":"awilliam@redhat.com"},"change_message_id":"18a4e38a90addc95693e554ecddfcc937608fc2c","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        lines \u003d textwrap.dedent(code).strip().splitlines(True)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        guide \u003d pep8.StyleGuide(select\u003d\u0027K333\u0027)"},{"line_number":43,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines, options\u003dguide.options)"},{"line_number":44,"context_line":"        checker.check_all()"},{"line_number":45,"context_line":"        checker.report._deferred_print.sort()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_17016805","line":42,"in_reply_to":"ba5201f7_316afcee","updated":"2017-01-05 19:48:51.000000000","message":"Ug, and now I look at it more, there\u0027s a whole world of idiosyncracies in terms of how pycodestyle / pep8 \u0027select\u0027 and \u0027ignore\u0027 work, and how it decides which codes are associated with which checks. Basically, you can pass the codes in as an arg when you call `register_check`; if you don\u0027t, and the check is a function, it will look in the check\u0027s docstring for any string that matches a regex designed to find code-ish strings - ERRORCODE_REGEX \u003d re.compile(r\u0027\\b[A-Z]\\d{3}\\b\u0027) , so best hope you never need more than 999 codes! - and if it gets any matches, it\u0027ll use them as the list of codes.\n\nIf you don\u0027t pass any codes to `register_check`, and the check is a class not a function or the function\u0027s docstring doesn\u0027t contain any codes, then the check will be registered with *no* associated codes - and due to how the \u0027select\u0027 code works, all checks with no associated codes will always be selected if any \u0027select\u0027 value is specified. In fact that whole area in pycodestyle (look at the `StyleGuide.ignore_code` method, if you\u0027re interested) is rather weird and janky and doesn\u0027t seem to know whether it wants the \u0027select\u0027 and \u0027ignore\u0027 options to be iterables or strings.\n\nIn other words, this whole thing is a mess and I hate it all and you may as well just keep your mock certainly in keystone, as no-one\u0027s paying me to spend a week cleaning this up. Now I dug into this stuff even more, I can see that my patch for python-keystoneclient is also slightly incorrect and is only actually working on a technicality (the \u0027all checks with no code are selected if any \u0027select\u0027 value is specified\u0027 technicality, as the check gets registered with no code the way things are currently written). I\u0027ll fix up the patch for python-keystoneclient, but it\u0027d be rather more work to make this change appropriate for keystone and I don\u0027t think I wanna do that work.","commit_id":"bae3dce8ab58e6bfd716fb59f8f3accecf2acf35"},{"author":{"_account_id":24681,"name":"Adam Williamson","email":"awilliam@redhat.com"},"change_message_id":"80963a63f7ba6ff66a9a7903c7e657c45ad3d7c1","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        lines \u003d textwrap.dedent(code).strip().splitlines(True)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        guide \u003d pep8.StyleGuide(select\u003d\u0027K333\u0027)"},{"line_number":43,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines, options\u003dguide.options)"},{"line_number":44,"context_line":"        checker.check_all()"},{"line_number":45,"context_line":"        checker.report._deferred_print.sort()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_316afcee","line":42,"in_reply_to":"ba5201f7_51b22007","updated":"2017-01-05 19:15:08.000000000","message":"OK, yeah, I went and looked at the actual checks that derive from this base class, and you\u0027d have to do a bit more work to make this change make sense in this context. You\u0027d need a way to conveniently get the appropriate check codes for each check class (including the ones which can return *multiple* codes, like CheckForAssertingNoneEquality , which produces K003 and K004).\n\nI still don\u0027t understand why Steve took my patch for python-keystoneclient and made it into a patch for keystone, but hey.","commit_id":"bae3dce8ab58e6bfd716fb59f8f3accecf2acf35"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"711001f13620f2a074c1a9d34e30736e10a90370","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        lines \u003d textwrap.dedent(code).strip().splitlines(True)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        guide \u003d pep8.StyleGuide(select\u003d\u0027K333\u0027)"},{"line_number":43,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines, options\u003dguide.options)"},{"line_number":44,"context_line":"        checker.check_all()"},{"line_number":45,"context_line":"        checker.report._deferred_print.sort()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_0e1a397d","line":42,"in_reply_to":"ba5201f7_5fa23b9a","updated":"2017-01-05 18:51:11.000000000","message":"++","commit_id":"bae3dce8ab58e6bfd716fb59f8f3accecf2acf35"},{"author":{"_account_id":24681,"name":"Adam Williamson","email":"awilliam@redhat.com"},"change_message_id":"aedd2814c3cb08de1e161b1384a28e09986d9c64","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        lines \u003d textwrap.dedent(code).strip().splitlines(True)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        guide \u003d pep8.StyleGuide(select\u003d\u0027K333\u0027)"},{"line_number":43,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines, options\u003dguide.options)"},{"line_number":44,"context_line":"        checker.check_all()"},{"line_number":45,"context_line":"        checker.report._deferred_print.sort()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_dc570e58","line":42,"range":{"start_line":42,"start_character":40,"end_line":42,"end_character":44},"in_reply_to":"ba5201f7_5fa23b9a","updated":"2017-01-05 19:08:50.000000000","message":"it\u0027s the check\u0027s name/code for pep8/pycodestyle/flake8. It\u0027s already just included as a string in the actual check definition, and like 20 times in the `expected_errors` list. Making it into a variable seems like it would better be done as a separate commit.","commit_id":"bae3dce8ab58e6bfd716fb59f8f3accecf2acf35"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"2103ff9ae63a489e501071b082039bed004b5ba1","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines)"},{"line_number":48,"context_line":"        checker.check_all()"},{"line_number":49,"context_line":"        checker.report._deferred_print.sort()"},{"line_number":50,"context_line":"        return checker.report._deferred_print"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def assert_has_errors(self, code, expected_errors\u003dNone):"},{"line_number":53,"context_line":"        actual_errors \u003d [e[:3] for e in self.run_check(code)]"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_375e0877","side":"PARENT","line":50,"updated":"2017-01-09 17:28:50.000000000","message":"It would be awesome to get rid of this hack too.","commit_id":"937a1a3037c456cd111cac399f09647a38136b91"},{"author":{"_account_id":24681,"name":"Adam Williamson","email":"awilliam@redhat.com"},"change_message_id":"32dc83f70219357732072914607bb9faf7691adb","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        checker \u003d pep8.Checker(lines\u003dlines)"},{"line_number":48,"context_line":"        checker.check_all()"},{"line_number":49,"context_line":"        checker.report._deferred_print.sort()"},{"line_number":50,"context_line":"        return checker.report._deferred_print"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def assert_has_errors(self, code, expected_errors\u003dNone):"},{"line_number":53,"context_line":"        actual_errors \u003d [e[:3] for e in self.run_check(code)]"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_2951500f","side":"PARENT","line":50,"in_reply_to":"ba5201f7_375e0877","updated":"2017-01-09 17:34:56.000000000","message":"yeah, I did look at that one briefly but couldn\u0027t see a better way unfortunately :/ maybe an upstream PR / issue to get more flexible access to the check process...","commit_id":"937a1a3037c456cd111cac399f09647a38136b91"}]}