54acf0e96a
Patch Set 8: Code-Review-1
(7 comments)
Patch-set: 8
Reviewer: Gerrit User 36476 <36476@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1, 3bb087e410fa41e33761cc29316d86596582af3a
Attention: {"person_ident":"Gerrit User 36476 \u003c36476@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_36476\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 35809 \u003c35809@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_36476\u003e replied on the change"}
153 lines
4.6 KiB
Plaintext
153 lines
4.6 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "e66b5bf2_7f2ceaf1",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 8
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 36476
|
|
},
|
|
"writtenOn": "2024-03-20T13:18:43Z",
|
|
"side": 1,
|
|
"message": "Please, review the code and remove the duplications to simplify the testing",
|
|
"revId": "8284580c431615b7c19c6ebf956f6b5ff66a7a64",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "ae5e255b_c0ec41ec",
|
|
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
|
|
"patchSetId": 8
|
|
},
|
|
"lineNbr": 68,
|
|
"author": {
|
|
"id": 36476
|
|
},
|
|
"writtenOn": "2024-03-20T13:18:43Z",
|
|
"side": 1,
|
|
"message": "Create a single method to create the fake sites instead of using multiple variables and duplicating the code",
|
|
"range": {
|
|
"startLine": 64,
|
|
"startChar": 0,
|
|
"endLine": 68,
|
|
"endChar": 64
|
|
},
|
|
"revId": "8284580c431615b7c19c6ebf956f6b5ff66a7a64",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "9b310c05_26ece59c",
|
|
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
|
|
"patchSetId": 8
|
|
},
|
|
"lineNbr": 138,
|
|
"author": {
|
|
"id": 36476
|
|
},
|
|
"writtenOn": "2024-03-20T13:18:43Z",
|
|
"side": 1,
|
|
"message": "All of the classes above should be unnecessary. As long as the mocks are created, you can create the same behaviour without adding the complexity of using these",
|
|
"range": {
|
|
"startLine": 84,
|
|
"startChar": 1,
|
|
"endLine": 138,
|
|
"endChar": 8
|
|
},
|
|
"revId": "8284580c431615b7c19c6ebf956f6b5ff66a7a64",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "73e468c5_5e4df037",
|
|
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
|
|
"patchSetId": 8
|
|
},
|
|
"lineNbr": 145,
|
|
"author": {
|
|
"id": 36476
|
|
},
|
|
"writtenOn": "2024-03-20T13:18:43Z",
|
|
"side": 1,
|
|
"message": "In super, you don\u0027t need to specify the class and self. Opt for super().setUp().\nThis comment also applies to the other classes",
|
|
"range": {
|
|
"startLine": 145,
|
|
"startChar": 8,
|
|
"endLine": 145,
|
|
"endChar": 50
|
|
},
|
|
"revId": "8284580c431615b7c19c6ebf956f6b5ff66a7a64",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "c479cdba_97aab1ba",
|
|
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
|
|
"patchSetId": 8
|
|
},
|
|
"lineNbr": 287,
|
|
"author": {
|
|
"id": 36476
|
|
},
|
|
"writtenOn": "2024-03-20T13:18:43Z",
|
|
"side": 1,
|
|
"message": "Is it really necessary to mock utils? Can\u0027t the behavior be replicated without it? This will reduce its coverage",
|
|
"revId": "8284580c431615b7c19c6ebf956f6b5ff66a7a64",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "ba611f90_b2bf2773",
|
|
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
|
|
"patchSetId": 8
|
|
},
|
|
"lineNbr": 289,
|
|
"author": {
|
|
"id": 36476
|
|
},
|
|
"writtenOn": "2024-03-20T13:18:43Z",
|
|
"side": 1,
|
|
"message": "Use a single mock for these classes using the same structure from base.",
|
|
"range": {
|
|
"startLine": 288,
|
|
"startChar": 0,
|
|
"endLine": 289,
|
|
"endChar": 62
|
|
},
|
|
"revId": "8284580c431615b7c19c6ebf956f6b5ff66a7a64",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "a54856fb_ef38d3e8",
|
|
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
|
|
"patchSetId": 8
|
|
},
|
|
"lineNbr": 339,
|
|
"author": {
|
|
"id": 36476
|
|
},
|
|
"writtenOn": "2024-03-20T13:18:43Z",
|
|
"side": 1,
|
|
"message": "All of these mocks specifications are duplicated in most test cases, use the base.py structure in order to simplify this.",
|
|
"range": {
|
|
"startLine": 329,
|
|
"startChar": 8,
|
|
"endLine": 339,
|
|
"endChar": 56
|
|
},
|
|
"revId": "8284580c431615b7c19c6ebf956f6b5ff66a7a64",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |