charms.ceph/10663e5bd39e971810806d173d0ab959619cb19a

{
  "comments": [
    {
      "key": {
        "uuid": "120a1903_b38cdeda",
        "filename": "charms_ceph/utils.py",
        "patchSetId": 1
      },
      "lineNbr": 521,
      "author": {
        "id": 20870
      },
      "writtenOn": "2021-03-31T10:51:02Z",
      "side": 1,
      "message": "No, this is not really a good way to solve this problem.  Rather than using **kwargs, please use the standard pattern of osd\u003dNone, host\u003dNone, etc. and then in the body of __init__() do:\n\n    self.osd \u003d osd or \"\"\n    self.host \u003d host or \"\"\n\nThe reason is, is that people with type checking editors won\u0027t get assistance when writing code, and you can\u0027t look at the function signature to see what the function takes.  Plus, if a caller function mispells a kwarg it won\u0027t get flagged as an error.\n\ne.g. CrushLocation(hosty\u003d\"thing\") would not end up with self.host set, NOR would an error be raised on the caller side.",
      "range": {
        "startLine": 507,
        "startChar": 1,
        "endLine": 521,
        "endChar": 42
      },
      "revId": "10663e5bd39e971810806d173d0ab959619cb19a",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    },
    {
      "key": {
        "uuid": "6db7a0df_27a8f221",
        "filename": "charms_ceph/utils.py",
        "patchSetId": 1
      },
      "lineNbr": 521,
      "author": {
        "id": 32363
      },
      "writtenOn": "2021-04-01T10:13:08Z",
      "side": 1,
      "message": "Done",
      "parentUuid": "120a1903_b38cdeda",
      "range": {
        "startLine": 507,
        "startChar": 1,
        "endLine": 521,
        "endChar": 42
      },
      "revId": "10663e5bd39e971810806d173d0ab959619cb19a",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": false
    },
    {
      "key": {
        "uuid": "5ad975ba_ec1a6e63",
        "filename": "charms_ceph/utils.py",
        "patchSetId": 1
      },
      "lineNbr": 609,
      "author": {
        "id": 20870
      },
      "writtenOn": "2021-03-31T10:51:02Z",
      "side": 1,
      "message": "This should be and underscore (_) if the variable isn\u0027t needed in the comprehension.\n\nAlso, isn\u0027t this just:\n\n    results \u003d list(nodes.values())\n\n??",
      "range": {
        "startLine": 609,
        "startChar": 24,
        "endLine": 609,
        "endChar": 32
      },
      "revId": "10663e5bd39e971810806d173d0ab959619cb19a",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    },
    {
      "key": {
        "uuid": "a7f7b343_0bd859a9",
        "filename": "charms_ceph/utils.py",
        "patchSetId": 1
      },
      "lineNbr": 609,
      "author": {
        "id": 32363
      },
      "writtenOn": "2021-04-01T10:13:08Z",
      "side": 1,
      "message": "You\u0027re right, I don\u0027t know why it\u0027s there, maybe copypasta. No, because we need to filter nodes by their types.\n    results \u003d [node for node in nodes.values() if node.get(\"type\") \u003d\u003d node_type]",
      "parentUuid": "5ad975ba_ec1a6e63",
      "range": {
        "startLine": 609,
        "startChar": 24,
        "endLine": 609,
        "endChar": 32
      },
      "revId": "10663e5bd39e971810806d173d0ab959619cb19a",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    },
    {
      "key": {
        "uuid": "23758c87_b4a15743",
        "filename": "charms_ceph/utils.py",
        "patchSetId": 1
      },
      "lineNbr": 621,
      "author": {
        "id": 20870
      },
      "writtenOn": "2021-03-31T10:51:02Z",
      "side": 1,
      "message": "Is this a typo?  Are you intended an in-place mutation of the elements of results using the recursive append_parent() function?",
      "range": {
        "startLine": 620,
        "startChar": 0,
        "endLine": 621,
        "endChar": 44
      },
      "revId": "10663e5bd39e971810806d173d0ab959619cb19a",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    },
    {
      "key": {
        "uuid": "82966329_f9c5f490",
        "filename": "charms_ceph/utils.py",
        "patchSetId": 1
      },
      "lineNbr": 621,
      "author": {
        "id": 32363
      },
      "writtenOn": "2021-04-01T10:13:08Z",
      "side": 1,
      "message": "I see that this is a really bad solution.",
      "parentUuid": "23758c87_b4a15743",
      "range": {
        "startLine": 620,
        "startChar": 0,
        "endLine": 621,
        "endChar": 44
      },
      "revId": "10663e5bd39e971810806d173d0ab959619cb19a",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    },
    {
      "key": {
        "uuid": "6d886228_4721432f",
        "filename": "charms_ceph/utils.py",
        "patchSetId": 1
      },
      "lineNbr": 646,
      "author": {
        "id": 20870
      },
      "writtenOn": "2021-03-31T10:51:02Z",
      "side": 1,
      "message": "I get why this is being done, but really, the written out version is much easier to read what is going on.",
      "range": {
        "startLine": 645,
        "startChar": 0,
        "endLine": 646,
        "endChar": 51
      },
      "revId": "10663e5bd39e971810806d173d0ab959619cb19a",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    },
    {
      "key": {
        "uuid": "2fe3bd29_247e6769",
        "filename": "charms_ceph/utils.py",
        "patchSetId": 1
      },
      "lineNbr": 646,
      "author": {
        "id": 32363
      },
      "writtenOn": "2021-04-01T10:13:08Z",
      "side": 1,
      "message": "I rewrote the whole parse host information from the tree and I think now it is more readable and the outputs are clearer.\n\nI know the output is more readable, but I need this information to get information about AZ. Hopefully you will like my new approach more.",
      "parentUuid": "6d886228_4721432f",
      "range": {
        "startLine": 645,
        "startChar": 0,
        "endLine": 646,
        "endChar": 51
      },
      "revId": "10663e5bd39e971810806d173d0ab959619cb19a",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    }
  ]
}