292 lines
11 KiB
Plaintext
292 lines
11 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "49d0bfa2_e1b8c956",
|
|
"filename": "/COMMIT_MSG",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 8,
|
|
"author": {
|
|
"id": 20870
|
|
},
|
|
"writtenOn": "2024-01-05T12:58:06Z",
|
|
"side": 1,
|
|
"message": "This commit message needs expanding, please, with a description of *why* the feature is being added, *how* it impacts usage, and *what* effects it produces. e.g. it overrides the \"crl_distribution_points\" parameter in the \"client.secrets.pki.set_urls(...)\" call to vault.\n\nIs there a related bug to this feature. If so, and this patch fully resolves the feature/bug, then please add:\n\nCloses-bug: #\u003cLP bug number\u003e\n\nThe purpose of the commit message is to allow a reader of the patch to understand why the patch is being added, what impact it has, and any implications for existing installations.",
|
|
"range": {
|
|
"startLine": 6,
|
|
"startChar": 0,
|
|
"endLine": 8,
|
|
"endChar": 0
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "16e3aed9_d46b80ff",
|
|
"filename": "/COMMIT_MSG",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 8,
|
|
"author": {
|
|
"id": 20870
|
|
},
|
|
"writtenOn": "2024-01-08T13:24:00Z",
|
|
"side": 1,
|
|
"message": "This commit message still needs work. Just pointing to a bug creates work for other people when they are viewing the commit message.\n\nPlease see the following for examples:\n\n- https://docs.opendev.org/opendev/infra-manual/latest/developers.html#committing-changes\n- Post on how to write a good commit message: https://cbea.ms/git-commit/\n\nSay what the commit does and why, please.\n\nPlease remove the \"Customer:\" part; this is an open source project and whilst the feature addition may have been prompted due to a customer, it\u0027s still just a feature being added to this open source project.",
|
|
"parentUuid": "49d0bfa2_e1b8c956",
|
|
"range": {
|
|
"startLine": 6,
|
|
"startChar": 0,
|
|
"endLine": 8,
|
|
"endChar": 0
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "4e163bc2_ec9f7dd5",
|
|
"filename": "/COMMIT_MSG",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 8,
|
|
"author": {
|
|
"id": 36623
|
|
},
|
|
"writtenOn": "2024-01-17T20:07:22Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "16e3aed9_d46b80ff",
|
|
"range": {
|
|
"startLine": 6,
|
|
"startChar": 0,
|
|
"endLine": 8,
|
|
"endChar": 0
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "b0ead2bf_e47972b3",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 20870
|
|
},
|
|
"writtenOn": "2024-01-05T12:58:06Z",
|
|
"side": 1,
|
|
"message": "Thanks for the patch. However, it needs a little work to bring it up to mergeable status. Please review the comments below (and the inline comments).\n\nAs the patch is changing an existing function that has test coverage (upload_signed_csr()) it really also need some additional unit test coverage. This is to ensure that future changes to break this addition. i.e. the unit tests validate the contract that the functional signature offers, and this is changing the function signature of upload_signed_csr().\n\nI\u0027m holding of a charm-recheck as the patch will require additional changes. The failure was due to an underlying infra issue, and not related to the patch. Thanks.",
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "93a24837_71b28a0a",
|
|
"filename": "src/actions.yaml",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 110,
|
|
"author": {
|
|
"id": 20870
|
|
},
|
|
"writtenOn": "2024-01-05T13:00:49Z",
|
|
"side": 1,
|
|
"message": "Could this not be \"crl-distribution-point\" to more closely match the parameter in the vault call?",
|
|
"range": {
|
|
"startLine": 110,
|
|
"startChar": 4,
|
|
"endLine": 110,
|
|
"endChar": 18
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "2d178d69_3d99a670",
|
|
"filename": "src/actions.yaml",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 110,
|
|
"author": {
|
|
"id": 36623
|
|
},
|
|
"writtenOn": "2024-01-17T20:07:22Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "93a24837_71b28a0a",
|
|
"range": {
|
|
"startLine": 110,
|
|
"startChar": 4,
|
|
"endLine": 110,
|
|
"endChar": 18
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "c4a2db20_9b73746e",
|
|
"filename": "src/actions.yaml",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 112,
|
|
"author": {
|
|
"id": 20870
|
|
},
|
|
"writtenOn": "2024-01-05T12:58:06Z",
|
|
"side": 1,
|
|
"message": "trailing whitespace needs removing.",
|
|
"range": {
|
|
"startLine": 112,
|
|
"startChar": 0,
|
|
"endLine": 112,
|
|
"endChar": 18
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "ca78e4bf_c393e1d2",
|
|
"filename": "src/actions.yaml",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 112,
|
|
"author": {
|
|
"id": 36623
|
|
},
|
|
"writtenOn": "2024-01-17T20:07:22Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "c4a2db20_9b73746e",
|
|
"range": {
|
|
"startLine": 112,
|
|
"startChar": 0,
|
|
"endLine": 112,
|
|
"endChar": 18
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "b322089b_4fee06a9",
|
|
"filename": "src/lib/charm/vault_pki.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 206,
|
|
"author": {
|
|
"id": 20870
|
|
},
|
|
"writtenOn": "2024-01-05T12:58:06Z",
|
|
"side": 1,
|
|
"message": "need to add the :param ...: and :type ...: into the function\u0027s docstring to explain what the parameter is for, and the type s allowed. Note the existing ones are wrong.\n\nIn this case it should be:\n\n :pararm crl_dist_point: explain what it is\n :type crl_dist_point: Optional[str]\n \nBonus points if you update the existing types to include `Optional[\u003ctype\u003e]` for the optional parameters!",
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "d90ea1e8_e2babb72",
|
|
"filename": "src/lib/charm/vault_pki.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 206,
|
|
"author": {
|
|
"id": 20870
|
|
},
|
|
"writtenOn": "2024-01-05T13:00:49Z",
|
|
"side": 1,
|
|
"message": "This param would probably be better named as \"crl_distribution_point\" to more closely match the calling param in set_urls.\n\nAlso is there a reason it is singular? The set_urls() function appears to be expecting a list of urls?",
|
|
"range": {
|
|
"startLine": 206,
|
|
"startChar": 41,
|
|
"endLine": 206,
|
|
"endChar": 42
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "fa0f12ef_612c0767",
|
|
"filename": "src/lib/charm/vault_pki.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 206,
|
|
"author": {
|
|
"id": 935
|
|
},
|
|
"writtenOn": "2024-01-10T11:36:38Z",
|
|
"side": 1,
|
|
"message": "I had a think about the singularity question today - I think this is OK and reflects a potential deployment scenario where the Vault infrastructure is not directly accessible from client endpoints using the services that it has issued certificates for.\n\nHaving a global/enterprise CRL distribution point that is generally accessible makes sense in this case - however there is a manual process needed to sync certificates revoked in vault into whatever is managing this different distribution point.\n\nI\u0027ve suggested an updated to the action parameter description to explain this better.",
|
|
"parentUuid": "d90ea1e8_e2babb72",
|
|
"range": {
|
|
"startLine": 206,
|
|
"startChar": 41,
|
|
"endLine": 206,
|
|
"endChar": 42
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "43d77983_335334e5",
|
|
"filename": "src/lib/charm/vault_pki.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 206,
|
|
"author": {
|
|
"id": 36623
|
|
},
|
|
"writtenOn": "2024-01-17T20:07:22Z",
|
|
"side": 1,
|
|
"message": "Customer returned with a reply that \"We are fine with a single URI as CDP, as long as the URI could be resolved and reachable from all the systems we are fine with it.\"",
|
|
"parentUuid": "d90ea1e8_e2babb72",
|
|
"range": {
|
|
"startLine": 206,
|
|
"startChar": 41,
|
|
"endLine": 206,
|
|
"endChar": 42
|
|
},
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "f2e07fbb_339f009f",
|
|
"filename": "src/lib/charm/vault_pki.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 206,
|
|
"author": {
|
|
"id": 36623
|
|
},
|
|
"writtenOn": "2024-01-17T20:07:22Z",
|
|
"side": 1,
|
|
"message": "Hi Alex, this was smt I added but when I removed last commit I lost this change. Do you want me to do this \"Optional\" tag for all other actions also? Because in none Optional explanation was given.",
|
|
"parentUuid": "b322089b_4fee06a9",
|
|
"revId": "b2d1dddc27f8f71382a2ff68f56470b51dcdf986",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |