zuul
/
zuul-jobs
Code Issues Proposed changes

Update patch set 28 

Patch Set 28:

(2 comments)

Patch-set: 28
Attention: {"person_ident":"Gerrit User 36143 \u003c36143@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_36143\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 1 \u003c1@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_36143\u003e replied on the change"}
This commit is contained in:
Gerrit User 36143 2024-02-19 09:40:57 +00:00 committed by Gerrit Code Review
parent 0a2dcf3e23
commit 57346754b5
2 changed files with 36 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
3b3bd4d5943c44bd8c75f2003f13de81e6ebbbff
View File

@ -16,6 +16,24 @@
"message": "The old variable name wasn\u0027t great since you decided to sync required projects and also the project under test, but I think it was better than this. This makes me think that normally we don\u0027t sync required projects and you need to set this to true in order to do that. I think the old name was better (especially since we document the edge case below), but I\u0027m open to other ideas if you have them. I don\u0027t have a better suggestion.",
"revId": "3b3bd4d5943c44bd8c75f2003f13de81e6ebbbff",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": false,
"key": {
"uuid": "f44025f2_c54f2851",
"filename": "roles/prepare-workspace-git/README.rst",
"patchSetId": 28
},
"lineNbr": 19,
"author": {
"id": 36143
},
"writtenOn": "2024-02-19T09:40:57Z",
"side": 1,
"message": "Valid point, I would have one suggestion: `prepare_workspace_sync_selected_projects` since with this feature we want to give users the option to pick out the projects which they want to sync (except the project under test).\n\nHowever, we can also revert to `prepare_workspace_sync_required_projects_only` which is maybe more descriptive of what is happening (except the project under test).\n\nAlso I\u0027m not sure if we can omit the `prepare_workspace_` prefix, since we are using this variable only in this specific role ?",
"parentUuid": "1ddd5cd9_ceeb6438",
"revId": "3b3bd4d5943c44bd8c75f2003f13de81e6ebbbff",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}

18
4d4e9c8fb67f2bd8a0bbc7d518b94f34fc1ed421
View File

@ -88,6 +88,24 @@
"parentUuid": "47740a8a_a2c32842",
"revId": "4d4e9c8fb67f2bd8a0bbc7d518b94f34fc1ed421",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "48e264ff_c76eb838",
"filename": "test-playbooks/base-roles/prepare-workspace-git-required-projects-only.yaml",
"patchSetId": 25
},
"lineNbr": 33,
"author": {
"id": 36143
},
"writtenOn": "2024-02-19T09:40:57Z",
"side": 1,
"message": "I updated the commit message and the variable name as you already reviewed\nand yes syncing the project under test in all cases is a desired behaviour,\nas well as the possibility to sync the other projects only if they are required.\n\nYour suggestion to the test approach of adding the extra repo and overriding\nthe `required` flag of the repo sounds good to me and generally I would like to\nimplement it myself, however in this specific case feel free to add further \nchanges to the PR, since I won\u0027t have time for it in the next upcoming days.",
"parentUuid": "06ec8c48_f0b40cf3",
"revId": "4d4e9c8fb67f2bd8a0bbc7d518b94f34fc1ed421",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}