Update patch set 2
Patch Set 2: (4 comments) Can we get tests for this? Patch-set: 2 Label: Verified=0
This commit is contained in:
parent
88eb130d0b
commit
0c7d51331f
|
@ -0,0 +1,72 @@
|
|||
{
|
||||
"comments": [
|
||||
{
|
||||
"key": {
|
||||
"uuid": "7aa08908_3b1b321e",
|
||||
"filename": "giftwrap/builders/__init__.py",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 63,
|
||||
"author": {
|
||||
"id": 6530
|
||||
},
|
||||
"writtenOn": "2016-06-10T16:29:15Z",
|
||||
"side": 1,
|
||||
"message": "Nits, but not sure why there are lines between the code and prefer more descriptive variable names.",
|
||||
"revId": "c16cc483183290aa84071eae22d491573de469e7",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
|
||||
"unresolved": false
|
||||
},
|
||||
{
|
||||
"key": {
|
||||
"uuid": "7aa08908_bb0722c8",
|
||||
"filename": "giftwrap/builders/__init__.py",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 106,
|
||||
"author": {
|
||||
"id": 6530
|
||||
},
|
||||
"writtenOn": "2016-06-10T16:29:15Z",
|
||||
"side": 1,
|
||||
"message": "This will pull the URL for every project. Shouldn\u0027t this be per-build?",
|
||||
"revId": "c16cc483183290aa84071eae22d491573de469e7",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
|
||||
"unresolved": false
|
||||
},
|
||||
{
|
||||
"key": {
|
||||
"uuid": "7aa08908_5e20ac49",
|
||||
"filename": "giftwrap/builders/docker_builder.py",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 67,
|
||||
"author": {
|
||||
"id": 6530
|
||||
},
|
||||
"writtenOn": "2016-06-10T16:29:15Z",
|
||||
"side": 1,
|
||||
"message": "This is pretty good :) We should probably fix that.",
|
||||
"revId": "c16cc483183290aa84071eae22d491573de469e7",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
|
||||
"unresolved": false
|
||||
},
|
||||
{
|
||||
"key": {
|
||||
"uuid": "7aa08908_3bc5f23c",
|
||||
"filename": "giftwrap/builders/docker_builder.py",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 88,
|
||||
"author": {
|
||||
"id": 6530
|
||||
},
|
||||
"writtenOn": "2016-06-10T16:29:15Z",
|
||||
"side": 1,
|
||||
"message": "I\u0027d rather see this as a property of a Builder:\n\n@property\ndef uses_contraints(self):\n return self._constraints_path\n\nReason being is that there may be other directives that would invoke the use of \"-c\"",
|
||||
"revId": "c16cc483183290aa84071eae22d491573de469e7",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
|
||||
"unresolved": false
|
||||
}
|
||||
]
|
||||
}
|
Loading…
Reference in New Issue