)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9420,"name":"Matthew Gilliard","email":"matthew.gilliard@hp.com","username":"gilliard"},"change_message_id":"4a4b681dd3245714ea1ea1fd007460ad4b6a7c8d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Skip sqlite-specific tests if sqlite is not configured"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There are several tests in test_migration_utils which only work if"},{"line_number":10,"context_line":"sqlite is configured for migration tests, but two of them are missing"},{"line_number":11,"context_line":"the conditional checks for skipping the test."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Also renames test_check_shadow_table_with_unsupported_type to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAAVn%2F%2F488%3D","line":10,"updated":"2014-03-03 16:06:26.000000000","message":"I only see one missing the conditional check (test_check_shadow_table_with_unsupported_sqlite_type).  Did I misunderstand something?","commit_id":"c283576770b1bb96fd3dd012c6ced22b2a8eb903"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2215bf1eb34bbd581f5309dc9e948f768ef0fa26","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Skip sqlite-specific tests if sqlite is not configured"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There are several tests in test_migration_utils which only work if"},{"line_number":10,"context_line":"sqlite is configured for migration tests, but two of them are missing"},{"line_number":11,"context_line":"the conditional checks for skipping the test."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Also renames test_check_shadow_table_with_unsupported_type to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAAVn%2F%2F4KU%3D","line":10,"in_reply_to":"AAAAVn%2F%2F488%3D","updated":"2014-03-03 17:14:19.000000000","message":"Compare test_util_drop_unique_constraint_with_not_supported_sqlite_type in patch set 2 and patch set 3.","commit_id":"c283576770b1bb96fd3dd012c6ced22b2a8eb903"},{"author":{"_account_id":9420,"name":"Matthew Gilliard","email":"matthew.gilliard@hp.com","username":"gilliard"},"change_message_id":"73429dbe336906b4c86a63acbdbb8c440839d34b","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Skip sqlite-specific tests if sqlite is not configured"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There are several tests in test_migration_utils which only work if"},{"line_number":10,"context_line":"sqlite is configured for migration tests, but two of them are missing"},{"line_number":11,"context_line":"the conditional checks for skipping the test."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Also renames test_check_shadow_table_with_unsupported_type to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAAVn%2F%2F2eM%3D","line":10,"in_reply_to":"AAAAVn%2F%2F4KU%3D","updated":"2014-03-03 19:56:42.000000000","message":"OK - makes more sense now.  FWIW, I think the early return/skip is better too.  Even though it leads to a bigger diff the resulting code is cleaner, which is more important in the long run.","commit_id":"c283576770b1bb96fd3dd012c6ced22b2a8eb903"}],"nova/tests/db/test_migration_utils.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"071c5c0540f7ed321571273b5cefbe1c9402d43a","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            test_table.drop()"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def test_util_drop_unique_constraint_with_not_supported_sqlite_type(self):"},{"line_number":171,"context_line":"        if \u0027sqlite\u0027 in self.engines:"},{"line_number":172,"context_line":"            engine \u003d self.engines[\u0027sqlite\u0027]"},{"line_number":173,"context_line":"            meta \u003d MetaData(bind\u003dengine)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVX%2F%2F6KE%3D","line":170,"updated":"2014-02-27 13:08:57.000000000","message":"I think this should be done with a skip instead of just a giant nested if.","commit_id":"b8993e7838d8e8a00d82adfbac48dfb8e7032dfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cbd922df0512f8f177bcaea5b89aaf9a361f0628","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            test_table.drop()"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def test_util_drop_unique_constraint_with_not_supported_sqlite_type(self):"},{"line_number":171,"context_line":"        if \u0027sqlite\u0027 in self.engines:"},{"line_number":172,"context_line":"            engine \u003d self.engines[\u0027sqlite\u0027]"},{"line_number":173,"context_line":"            meta \u003d MetaData(bind\u003dengine)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVX%2F%2F3kU%3D","line":170,"in_reply_to":"AAAAVX%2F%2F4VQ%3D","updated":"2014-02-27 17:40:34.000000000","message":"BTW, you\u0027ll noticed there are a lot of tests in here that have a big nested if for sqlite-specific tests, but I\u0027ll clean this one up with skipTest.","commit_id":"b8993e7838d8e8a00d82adfbac48dfb8e7032dfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"24f460729ac2a2d8a23885f4bf9bbb3dc034dc85","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            test_table.drop()"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def test_util_drop_unique_constraint_with_not_supported_sqlite_type(self):"},{"line_number":171,"context_line":"        if \u0027sqlite\u0027 in self.engines:"},{"line_number":172,"context_line":"            engine \u003d self.engines[\u0027sqlite\u0027]"},{"line_number":173,"context_line":"            meta \u003d MetaData(bind\u003dengine)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAVX%2F%2F4VQ%3D","line":170,"in_reply_to":"AAAAVX%2F%2F6KE%3D","updated":"2014-02-27 16:18:26.000000000","message":"I thought about that originally but it would lead to a bigger diff, which I thought people might nit-pick....I can do it either way, I don\u0027t care.  I prefer the skip also.","commit_id":"b8993e7838d8e8a00d82adfbac48dfb8e7032dfa"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2f2d5e60f18c14d57c5c9dca6551c1fef6cce485","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"            test_table.drop()"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def test_util_drop_unique_constraint_with_not_supported_sqlite_type(self):"},{"line_number":169,"context_line":"        if \u0027sqlite\u0027 not in self.engines:"},{"line_number":170,"context_line":"            self.skipTest(\u0027sqlite is not configured\u0027)"},{"line_number":171,"context_line":"        engine \u003d self.engines[\u0027sqlite\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FEFg%3D","line":168,"updated":"2014-03-10 09:27:30.000000000","message":"not necessary to update all of these lines. you can add a else condition.","commit_id":"c283576770b1bb96fd3dd012c6ced22b2a8eb903"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"0d2ddbb5c793b3f53fd12446738cf2cc253801f5","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"            test_table.drop()"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def test_util_drop_unique_constraint_with_not_supported_sqlite_type(self):"},{"line_number":169,"context_line":"        if \u0027sqlite\u0027 not in self.engines:"},{"line_number":170,"context_line":"            self.skipTest(\u0027sqlite is not configured\u0027)"},{"line_number":171,"context_line":"        engine \u003d self.engines[\u0027sqlite\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FCI0%3D","line":168,"in_reply_to":"AAAAVn%2F%2FEAo%3D","updated":"2014-03-10 14:54:09.000000000","message":"yeh, this is much better as an explicit skip. And that was the direction given.","commit_id":"c283576770b1bb96fd3dd012c6ced22b2a8eb903"},{"author":{"_account_id":9420,"name":"Matthew Gilliard","email":"matthew.gilliard@hp.com","username":"gilliard"},"change_message_id":"90d682fd9aed0433ed593d236c5567c44012df69","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"            test_table.drop()"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def test_util_drop_unique_constraint_with_not_supported_sqlite_type(self):"},{"line_number":169,"context_line":"        if \u0027sqlite\u0027 not in self.engines:"},{"line_number":170,"context_line":"            self.skipTest(\u0027sqlite is not configured\u0027)"},{"line_number":171,"context_line":"        engine \u003d self.engines[\u0027sqlite\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FEAo%3D","line":168,"in_reply_to":"AAAAVn%2F%2FEFg%3D","updated":"2014-03-10 09:32:47.000000000","message":"I think this was discussed in an earlier patchset (see sdague\u0027s comment in PS2) - there\u0027s no harm in updating the lines, and the resulting code is cleaner with the early exit IMO.","commit_id":"c283576770b1bb96fd3dd012c6ced22b2a8eb903"}]}