Update patch set 2
Patch Set 2: -Code-Review (2 comments) Patch-set: 2 Label: Code-Review=0, d97d39d316275c7a5b869e2d99577a8f06228881
This commit is contained in:
parent
fcac8a25a0
commit
4785013e0a
|
@ -17,6 +17,52 @@
|
|||
"revId": "5394e96b456c4b0eeb67fcaa2044c0cc666970bc",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "5f76398d_b6e24de5",
|
||||
"filename": "build-tools/stx/patch/patch_builder.py",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 92,
|
||||
"author": {
|
||||
"id": 33390
|
||||
},
|
||||
"writtenOn": "2024-02-14T15:33:33Z",
|
||||
"side": 1,
|
||||
"message": "It would be better to declare these values as constants in the first lines of this module. Or maybe even better, have a dictionary mapping \"type\": \"filename\", like:\n\n```\nPRE_INSTALL \u003d \"preinstall\"\nPOST_INSTALL \u003d \"postinstall\"\nDEPLOY_PRECHECK \u003d \"deployprecheck\"\n\nPATCH_SCRIPTS \u003d {\n PRE_INSTALL: \"pre-install.sh\",\n POST_INSTALL: \"post-install.sh\",\n DEPLOY_PRECHECK: \"deploy-precheck\",\n}\n```\n\nThen you could summarize lines #122-133 into something like:\n\n```\nscript_name \u003d PATCH_SCRIPT.get(type, None)\nif script_name:\n log\n shutil.copy\nelse:\n raise ScriptTypeNotValid Exception\n```",
|
||||
"range": {
|
||||
"startLine": 92,
|
||||
"startChar": 29,
|
||||
"endLine": 92,
|
||||
"endChar": 41
|
||||
},
|
||||
"revId": "5394e96b456c4b0eeb67fcaa2044c0cc666970bc",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "5795bd54_b8bdb2a4",
|
||||
"filename": "build-tools/stx/patch/patch_builder.py",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 116,
|
||||
"author": {
|
||||
"id": 33390
|
||||
},
|
||||
"writtenOn": "2024-02-14T15:33:33Z",
|
||||
"side": 1,
|
||||
"message": "Should avoid using \"type\" as parameter name, it is the name of a python built-in function.",
|
||||
"range": {
|
||||
"startLine": 116,
|
||||
"startChar": 26,
|
||||
"endLine": 116,
|
||||
"endChar": 30
|
||||
},
|
||||
"revId": "5394e96b456c4b0eeb67fcaa2044c0cc666970bc",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
|
|
Loading…
Reference in New Issue