openstack
/
python-dracclient
Code Issues Proposed changes

Update patch set 3 

Patch Set 3:

(12 comments)

Thanks for patch. Some comments inline.

Patch-set: 3
Label: Verified=0
This commit is contained in:
Gerrit User 27909 2020-08-28 09:17:42 +00:00 committed by Gerrit Code Review
parent a290d2a63a
commit 46b57c8afc
1 changed files with 276 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

276
87b709a40dbf2aa5fa4c63c4031d82b680286c76
View File

@ -1,5 +1,189 @@
{
"comments": [
{
"key": {
"uuid": "9f560f44_71863816",
"filename": "/COMMIT_MSG",
"patchSetId": 3
},
"lineNbr": 7,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "Looking at the code, it looks more like that a title \"Add method for Last system inventory time\" describes better what is changed",
"range": {
"startLine": 7,
"startChar": 0,
"endLine": 7,
"endChar": 21
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_518cd42d",
"filename": "/COMMIT_MSG",
"patchSetId": 3
},
"lineNbr": 9,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "Returns This property -\u003e Returns property that ?",
"range": {
"startLine": 9,
"startChar": 0,
"endLine": 9,
"endChar": 21
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_51b5f4cf",
"filename": "/COMMIT_MSG",
"patchSetId": 3
},
"lineNbr": 10,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "Looking at the code I see that `LastSystemInventoryTime` is used. From where does \"System Inventory collection On Reboot(CSIOR)\" come? How are they related?",
"range": {
"startLine": 9,
"startChar": 45,
"endLine": 10,
"endChar": 27
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_11753c33",
"filename": "/COMMIT_MSG",
"patchSetId": 3
},
"lineNbr": 14,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "I\u0027d say this is irrelevant to this patch where this is going to be used. It can be used in more scenarios than listed here and planned now. Can remove this.",
"range": {
"startLine": 13,
"startChar": 0,
"endLine": 14,
"endChar": 38
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_d1c8e457",
"filename": "/COMMIT_MSG",
"patchSetId": 3
},
"lineNbr": 18,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "To be clarified, if this is necessary at all for python-dracclient project or need to be replaced with smth else.",
"range": {
"startLine": 17,
"startChar": 0,
"endLine": 18,
"endChar": 10
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_91fdcc70",
"filename": "dracclient/client.py",
"patchSetId": 3
},
"lineNbr": 99,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "same as in commit message - need to clarify this text.",
"range": {
"startLine": 98,
"startChar": 11,
"endLine": 99,
"endChar": 50
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_9132ec5d",
"filename": "dracclient/client.py",
"patchSetId": 3
},
"lineNbr": 100,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "Don\u0027t need to repeat this if it is already in :returns: section.",
"range": {
"startLine": 99,
"startChar": 51,
"endLine": 100,
"endChar": 26
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_f1ed28b8",
"filename": "dracclient/client.py",
"patchSetId": 3
},
"lineNbr": 101,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "better say what is returned, \"value is\" does not read well + \"value\" is too generic, for example, could use - Last system inventory time",
"range": {
"startLine": 101,
"startChar": 18,
"endLine": 101,
"endChar": 56
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_86f0f4ff",
@ -18,6 +202,52 @@
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_910b0c7c",
"filename": "dracclient/client.py",
"patchSetId": 3
},
"lineNbr": 108,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "parse this to python datetime here instead of returning string in format? Otherwise in most cases consumers of this method will have to parse it anyway and/or format in different date format.",
"range": {
"startLine": 108,
"startChar": 15,
"endLine": 108,
"endChar": 60
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_31084090",
"filename": "dracclient/client.py",
"patchSetId": 3
},
"lineNbr": 907,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "here and below need to revert this change as not related to the patch",
"range": {
"startLine": 906,
"startChar": 0,
"endLine": 907,
"endChar": 13
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_66f5e0ed",
@ -54,6 +284,52 @@
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_111ffcb7",
"filename": "dracclient/resources/system.py",
"patchSetId": 3
},
"lineNbr": 64,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "same comments as elsewhere",
"range": {
"startLine": 62,
"startChar": 12,
"endLine": 64,
"endChar": 23
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_b121f0ff",
"filename": "dracclient/resources/system.py",
"patchSetId": 3
},
"lineNbr": 66,
"author": {
"id": 27909
},
"writtenOn": "2020-08-28T09:17:42Z",
"side": 1,
"message": "is it the time when machine was last rebooted? Or Last system inventory time?",
"range": {
"startLine": 66,
"startChar": 18,
"endLine": 66,
"endChar": 60
},
"revId": "87b709a40dbf2aa5fa4c63c4031d82b680286c76",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_a6d6d852",