)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10980,"name":"Doug Wiegley","email":"dougwig@parkside.io","username":"dougw"},"change_message_id":"a312df771569630688e014a8f575ddd092324276","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2014-12-16 11:03:11 +1100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add a custom pylint check for oslo i18n and logging"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change adds a new pylint test capable of detecting numerous"},{"line_number":10,"context_line":"violations of OpenStack i18n policies:"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a961159_7c8985c9","line":8,"updated":"2014-12-18 06:22:12.000000000","message":"Out of curiosity, why pylint and not hacking?","commit_id":"e75f310e8d57b8edd0e9709843f5bae0febf0585"},{"author":{"_account_id":10980,"name":"Doug Wiegley","email":"dougwig@parkside.io","username":"dougw"},"change_message_id":"65dabfc65f02376244fb675d991333a408a92230","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2014-12-16 11:03:11 +1100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add a custom pylint check for oslo i18n and logging"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change adds a new pylint test capable of detecting numerous"},{"line_number":10,"context_line":"violations of OpenStack i18n policies:"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a961159_869690ee","line":8,"in_reply_to":"3a961159_464668c1","updated":"2014-12-22 03:56:48.000000000","message":"Thanks, but I was unclear. I get that pylint is better. I\u0027m wondering if it\u0027s time to stop adding stuff to hacking, rather than having these two things living in parallel, with some odd contortions going on in hacking that would better live here. And if pylint is a good new target for these kinds of checks...","commit_id":"e75f310e8d57b8edd0e9709843f5bae0febf0585"},{"author":{"_account_id":11279,"name":"Angus Lees","email":"gus@inodes.org","username":"gus"},"change_message_id":"f0c6c7b4a12351c4d61ffba14919207e8a069c33","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2014-12-16 11:03:11 +1100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add a custom pylint check for oslo i18n and logging"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change adds a new pylint test capable of detecting numerous"},{"line_number":10,"context_line":"violations of OpenStack i18n policies:"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a961159_464668c1","line":8,"in_reply_to":"3a961159_7c8985c9","updated":"2014-12-22 03:38:00.000000000","message":"Two two are so different that it\u0027s quite unfair to compare them:\n\n- hacking allows you to write regex patterns to match against source code, or a slightly cleaned up version of source code (multiline statements converted into one line, leading/trailing whitespace removed, etc).\n\n- pylint builds a full syntax tree of the parsed source code, and allows you to write tests against the AST.  It also includes a surprisingly powerful \"inference engine\" that allows you to chase values back to their earlier definitions, etc.\n\nAs a simple example, consider this statement:\n\n   logging.debug(\"%s failed with %s\", (a, b))\n\nSimply counting the number of arguments to that function is awkward with hacking.  You need to write a function that uses regexes to count nested parentheses, and in the truly generic case correctly ignores pathological string values like \"\\\"(\\\"\".\nWith pylint, parsing the python code is all done for you and counting the arguments is as simple as len(node.func.args).","commit_id":"e75f310e8d57b8edd0e9709843f5bae0febf0585"},{"author":{"_account_id":11279,"name":"Angus Lees","email":"gus@inodes.org","username":"gus"},"change_message_id":"c272daeb925cbc92057c27c3d44d971728b43317","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2014-12-16 11:03:11 +1100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add a custom pylint check for oslo i18n and logging"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change adds a new pylint test capable of detecting numerous"},{"line_number":10,"context_line":"violations of OpenStack i18n policies:"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a961159_6aa7900b","line":8,"in_reply_to":"3a961159_869690ee","updated":"2014-12-22 22:54:42.000000000","message":"You can also write pylint checks that work on the raw source file, so yes pylint can very easily replace all uses of hacking and I strongly believe we should be building our static analysis checks on pylint.\n\nThe downside with pylint is that its defaults are noisy (see the long list of disables in .pylintrc) and it is GPL.  Unfortunately, the latter is causing some issues with using it in OpenStack, because it isn\u0027t clear where the boundaries of GPL apply in python.  So far we\u0027ve just avoided the issue but I\u0027m trying to get people to actually talk about it - and I suggest we don\u0027t rush to pylint until that\u0027s sorted out.","commit_id":"e75f310e8d57b8edd0e9709843f5bae0febf0585"}],"neutron/pylint/oslologcheck.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"11e49dcde3df7987e0f07dc16d41855ac2c6979e","unresolved":false,"context_lines":[{"line_number":15,"context_line":"import re"},{"line_number":16,"context_line":"import six"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import astroid"},{"line_number":19,"context_line":"from astroid import MANAGER"},{"line_number":20,"context_line":"from pylint import checkers"},{"line_number":21,"context_line":"from pylint.checkers import utils"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_3734a449","line":18,"updated":"2014-12-30 08:21:13.000000000","message":"Where does the module come from? I don\u0027t see anything relevant in requirements.txt or test-requirements.txt. Is it something that is pulled by pylint, and if so, shouldn\u0027t we explicitly add the dependency to test-requirements.txt to make sure it\u0027s present in virtualenv even when/if pylint does not pull it?","commit_id":"e75f310e8d57b8edd0e9709843f5bae0febf0585"},{"author":{"_account_id":11279,"name":"Angus Lees","email":"gus@inodes.org","username":"gus"},"change_message_id":"f783708c5f276d82b724d6e5e0f3b4fe36dc4e41","unresolved":false,"context_lines":[{"line_number":15,"context_line":"import re"},{"line_number":16,"context_line":"import six"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import astroid"},{"line_number":19,"context_line":"from astroid import MANAGER"},{"line_number":20,"context_line":"from pylint import checkers"},{"line_number":21,"context_line":"from pylint.checkers import utils"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_84a353b0","line":18,"in_reply_to":"3a961159_3734a449","updated":"2014-12-31 03:41:21.000000000","message":"The conclusion the last time this came up was that it wasn\u0027t necessary.  Discussion:\nhttps://review.openstack.org/#/c/120320/2/neutron/pylint/usagecheck.py\n\nHaving said that, it would also be good to not revisit this every time a similar change comes along.  Adding astroid explicitly to test-requirements.txt seems like an easy way to avoid surprising reviewers...\n\nAdded astroid to test-requirements.txt in the parent change.","commit_id":"e75f310e8d57b8edd0e9709843f5bae0febf0585"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9916a1e5451925ec223647af963bf6874de52df3","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        if not is_translated_const(node):"},{"line_number":209,"context_line":"            return"},{"line_number":210,"context_line":"        try:"},{"line_number":211,"context_line":"            keys, num_posargs \u003d utils.parse_format_string(node.value)"},{"line_number":212,"context_line":"            if ((keys and num_posargs !\u003d 0) or"},{"line_number":213,"context_line":"                num_posargs \u003e 1):"},{"line_number":214,"context_line":"                self.add_message(\u0027log-i18n-posargs\u0027, node\u003dnode)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_3cb1448f","line":211,"updated":"2015-01-08 10:39:08.000000000","message":"can num_posargs ever be negative? if not, you could just shortcut the following check to \u0027if num_posargs\u0027","commit_id":"5abe7f48998100e04a0f063c1da7425e19777066"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"d933b303bbd53d9c5466e3f2ae65eae7520846d8","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        if not is_translated_const(node):"},{"line_number":209,"context_line":"            return"},{"line_number":210,"context_line":"        try:"},{"line_number":211,"context_line":"            keys, num_posargs \u003d utils.parse_format_string(node.value)"},{"line_number":212,"context_line":"            if ((keys and num_posargs !\u003d 0) or"},{"line_number":213,"context_line":"                num_posargs \u003e 1):"},{"line_number":214,"context_line":"                self.add_message(\u0027log-i18n-posargs\u0027, node\u003dnode)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba7be1f8_d4da92e8","line":211,"in_reply_to":"1a930d6b_2ab6d3fa","updated":"2015-02-20 12:42:00.000000000","message":"I see. I think I thought that it was \u003e\u003d 1 when I wrote that. Sorry about that. What you said makes sense.","commit_id":"5abe7f48998100e04a0f063c1da7425e19777066"},{"author":{"_account_id":11279,"name":"Angus Lees","email":"gus@inodes.org","username":"gus"},"change_message_id":"f990bd037a47ae53b9778821642d33cba6d34b34","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        if not is_translated_const(node):"},{"line_number":209,"context_line":"            return"},{"line_number":210,"context_line":"        try:"},{"line_number":211,"context_line":"            keys, num_posargs \u003d utils.parse_format_string(node.value)"},{"line_number":212,"context_line":"            if ((keys and num_posargs !\u003d 0) or"},{"line_number":213,"context_line":"                num_posargs \u003e 1):"},{"line_number":214,"context_line":"                self.add_message(\u0027log-i18n-posargs\u0027, node\u003dnode)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a930d6b_2ab6d3fa","line":211,"in_reply_to":"3a961159_3cb1448f","updated":"2015-01-23 03:58:54.000000000","message":"num_posargs is never negative.  However, the below check isn\u0027t the same as \"if num_posargs\" - note num_posargs\u003d\u003d1 falls through...\n\nSo you can double-check my logic:\n\nparse_format_string() docstring:\n\"\"\"Parses a format string, returning a tuple of (keys, num_args), where keys is the set of mapping keys in the format string, and num_args is the number of arguments required by the format string.\"\"\"\n\n(Note the second return value isn\u0027t really the number of positional arguments in the format, but it\u0027s close enough since we don\u0027t use variable precision or field widths in translated strings.)\n\nI want the following test to accept the case where there are no positional arguments, or exactly one position argument and no named arguments - and to add log-i18n-posargs error for everything else.","commit_id":"5abe7f48998100e04a0f063c1da7425e19777066"}],"neutron/tests/unit/pylint/test_oslologcheck.py":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9916a1e5451925ec223647af963bf6874de52df3","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        self.assertEqual(node.lineno, msgs[0].node.lineno)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def test_unknowable(self):"},{"line_number":206,"context_line":"        # Should this be rejected as poor style?"},{"line_number":207,"context_line":"        tree \u003d test_utils.build_module(\"\"\""},{"line_number":208,"context_line":"        from neutron.openstack.common import log as logging"},{"line_number":209,"context_line":"        LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_5cac5035","line":206,"updated":"2015-01-08 10:39:08.000000000","message":"probably not. sometimes we return them to the user","commit_id":"5abe7f48998100e04a0f063c1da7425e19777066"}]}