Update patch set 2

Patch Set 2: Code-Review+1

(3 comments)

Thanks for the patch!!!!

I don't +2, not because of the inline comments, but because I think we should merge the patch that add CI jobs [1] and then run the tests on this patch.

[1]: https://review.openstack.org/#/c/638442/

Patch-set: 2
Label: Code-Review=+1
This commit is contained in:
Gerrit User 9535 2019-02-26 16:24:13 +00:00 committed by Gerrit Code Review
parent 81227e58ab
commit a3ff61fe77
1 changed files with 61 additions and 0 deletions

View File

@ -0,0 +1,61 @@
{
"comments": [
{
"key": {
"uuid": "9fdfeff1_4f09e981",
"filename": "/COMMIT_MSG",
"patchSetId": 2
},
"lineNbr": 14,
"author": {
"id": 9535
},
"writtenOn": "2019-02-26T16:24:13Z",
"side": 1,
"message": "nit: Would be nice to mention the change-id of the cinder patch that adds this feature (I3e7db26ef3df24a12e3bfa219fe25bfb315335ec).",
"revId": "27aa25ebafa20801e1c8ed3d451925a39a7e9ab8",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9fdfeff1_0f21a14d",
"filename": "cinderlib/cinderlib.py",
"patchSetId": 2
},
"lineNbr": 403,
"author": {
"id": 9535
},
"writtenOn": "2019-02-26T16:24:13Z",
"side": 1,
"message": "nit: I think it looks nicer with dict comprehension\n\n tmp_dict \u003d {k: str(v) for k, v in vars(opt).items()\n if not k.startswith(\u0027_\u0027)}\n\nWe could construct the whole `options` this way, but maybe it loses clarity:\n\n options \u003d [{k: str(v) for k, v in vars(opt).items()\n if not k.startswith(\u0027_\u0027)}\n for opt in oslo_options]",
"range": {
"startLine": 400,
"startChar": 0,
"endLine": 403,
"endChar": 62
},
"revId": "27aa25ebafa20801e1c8ed3d451925a39a7e9ab8",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9fdfeff1_af0975dd",
"filename": "cinderlib/cinderlib.py",
"patchSetId": 2
},
"lineNbr": 430,
"author": {
"id": 9535
},
"writtenOn": "2019-02-26T16:24:13Z",
"side": 1,
"message": "nit: I think it\u0027s clearer to see the join first (like it was) waiting for the other process to finish before checking the queue instead of just relying on the fact that `queue.get` defaults to blocking without timeout.",
"revId": "27aa25ebafa20801e1c8ed3d451925a39a7e9ab8",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
}
]
}