fff0f0b34f
Patch Set 7: Code-Review+2 (2 comments) Patch-set: 7 Reviewer: Gerrit User 8768 <8768@4a232e18-c5a9-48ee-94c0-e04e7cca6543> Label: Code-Review=+2, 4985cc56b808fa2a6d97046028d10a0e1472127c Attention: {"person_ident":"Gerrit User 8768 \u003c8768@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_8768\u003e replied on the change"}
210 lines
6.2 KiB
Plaintext
210 lines
6.2 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "70c88b07_7beeb719",
|
|
"filename": "/COMMIT_MSG",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 20,
|
|
"author": {
|
|
"id": 8768
|
|
},
|
|
"writtenOn": "2024-04-25T22:26:45Z",
|
|
"side": 1,
|
|
"message": "this seems like a pretty generic keyword to be used for the specific task of querying K8s version compatibility. Maybe something like \"sysinv-app query-k8s-version\" or \"sysinv-app query k8s-version\" would give more leeway in case we wanted to support other queries in the future?",
|
|
"range": {
|
|
"startLine": 20,
|
|
"startChar": 11,
|
|
"endLine": 20,
|
|
"endChar": 16
|
|
},
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "7f8511b7_f974c49e",
|
|
"filename": "/COMMIT_MSG",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 20,
|
|
"author": {
|
|
"id": 36077
|
|
},
|
|
"writtenOn": "2024-04-30T13:28:53Z",
|
|
"side": 1,
|
|
"message": "If another app query is added then it would make sense to update the arguments, but since there is only one at the moment I left it generic.",
|
|
"parentUuid": "70c88b07_7beeb719",
|
|
"range": {
|
|
"startLine": 20,
|
|
"startChar": 11,
|
|
"endLine": 20,
|
|
"endChar": 16
|
|
},
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "5831a277_72d3e1c2",
|
|
"filename": "/COMMIT_MSG",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 20,
|
|
"author": {
|
|
"id": 8768
|
|
},
|
|
"writtenOn": "2024-04-30T15:08:29Z",
|
|
"side": 1,
|
|
"message": "The downside of changing it later is that people have to change any notes or habits that they have. But I won\u0027t hold it up for this.",
|
|
"parentUuid": "7f8511b7_f974c49e",
|
|
"range": {
|
|
"startLine": 20,
|
|
"startChar": 11,
|
|
"endLine": 20,
|
|
"endChar": 16
|
|
},
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "2ea85a0d_493b5607",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 8768
|
|
},
|
|
"writtenOn": "2024-04-25T22:26:45Z",
|
|
"side": 1,
|
|
"message": "Generally looks okay, had a few comments. Other cores can feel free to merge if they think I\u0027m being too picky.",
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "1f32c91b_767b7aa3",
|
|
"filename": "sysinv/sysinv/sysinv/sysinv/cmd/applications.py",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 93,
|
|
"author": {
|
|
"id": 8768
|
|
},
|
|
"writtenOn": "2024-04-25T22:26:45Z",
|
|
"side": 1,
|
|
"message": "double comma intentional?",
|
|
"range": {
|
|
"startLine": 93,
|
|
"startChar": 42,
|
|
"endLine": 93,
|
|
"endChar": 44
|
|
},
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "254b3731_d6d78d5d",
|
|
"filename": "sysinv/sysinv/sysinv/sysinv/cmd/applications.py",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 93,
|
|
"author": {
|
|
"id": 36077
|
|
},
|
|
"writtenOn": "2024-04-30T13:28:53Z",
|
|
"side": 1,
|
|
"message": "Typo. Fixed.",
|
|
"parentUuid": "1f32c91b_767b7aa3",
|
|
"range": {
|
|
"startLine": 93,
|
|
"startChar": 42,
|
|
"endLine": 93,
|
|
"endChar": 44
|
|
},
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "35ed1aa1_5fefcbda",
|
|
"filename": "sysinv/sysinv/sysinv/sysinv/cmd/applications.py",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 162,
|
|
"author": {
|
|
"id": 8768
|
|
},
|
|
"writtenOn": "2024-04-25T22:26:45Z",
|
|
"side": 1,
|
|
"message": "can be simplified as:\n\ninclude_path \u003d \u0027--include-path\u0027 in sys.argv\n\nbut I think either one of those would also result in include_path being True if sys.argv includes something like \"--include-path-foo\".",
|
|
"range": {
|
|
"startLine": 162,
|
|
"startChar": 8,
|
|
"endLine": 162,
|
|
"endChar": 70
|
|
},
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "f78aa40a_f0cb94b9",
|
|
"filename": "sysinv/sysinv/sysinv/sysinv/cmd/applications.py",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 162,
|
|
"author": {
|
|
"id": 36077
|
|
},
|
|
"writtenOn": "2024-04-30T13:28:53Z",
|
|
"side": 1,
|
|
"message": "Six in one hand half a dozen in the other. A ternary at least makes it 100% the intent. I\u0027m going to leave it as is.",
|
|
"parentUuid": "35ed1aa1_5fefcbda",
|
|
"range": {
|
|
"startLine": 162,
|
|
"startChar": 8,
|
|
"endLine": 162,
|
|
"endChar": 70
|
|
},
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "307fde9a_61156b26",
|
|
"filename": "sysinv/sysinv/sysinv/sysinv/cmd/applications.py",
|
|
"patchSetId": 6
|
|
},
|
|
"lineNbr": 162,
|
|
"author": {
|
|
"id": 8768
|
|
},
|
|
"writtenOn": "2024-04-30T15:08:29Z",
|
|
"side": 1,
|
|
"message": "It seems like it might be possible to pass CONF.action.include_path into line 163, making use of the parser to interpret the argument.",
|
|
"parentUuid": "f78aa40a_f0cb94b9",
|
|
"range": {
|
|
"startLine": 162,
|
|
"startChar": 8,
|
|
"endLine": 162,
|
|
"endChar": 70
|
|
},
|
|
"revId": "9fc2be9eee926203734dba76feb2783d2aa6d2bd",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |