Update patch set 1
Patch Set 1: (1 comment) Patch-set: 1 Label: Workflow=0
This commit is contained in:
parent
f6136272ff
commit
d145d744e0
|
@ -0,0 +1,21 @@
|
|||
{
|
||||
"comments": [
|
||||
{
|
||||
"key": {
|
||||
"uuid": "3a461143_55ae9081",
|
||||
"filename": "ocf/NovaCompute",
|
||||
"patchSetId": 1
|
||||
},
|
||||
"lineNbr": 334,
|
||||
"author": {
|
||||
"id": 2394
|
||||
},
|
||||
"writtenOn": "2017-01-30T17:05:12Z",
|
||||
"side": 1,
|
||||
"message": "The change looks essentially fine to me but there\u0027s a nitpick which makes me pause before giving a +1: I\u0027m not too keen on how far the local variable declaration is split from its usage - that impacts readability and maintainability. You could instead put\n\n local validate_host\u003d1\n\njust before this if clause, and then just drop the else part. Declaring variables at the start of a function is usually just an unnecessary throwback to the ancient era of C programming before C99 :-)\n\nOf course, that wouldn\u0027t be a concern if this function wasn\u0027t already horribly overgrown, but even prior to any \"extract function\" refactoring, it makes sense to keep related code in the same part of the function.",
|
||||
"revId": "ccc7435cb16fb451fc8131a7b6abfc375e3a7f13",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
|
||||
"unresolved": false
|
||||
}
|
||||
]
|
||||
}
|
Loading…
Reference in New Issue