Add a cleanup step for bootstrapper files to remove stale NUKE_ENTERPRISE_TOKEN handling#519
Add a cleanup step for bootstrapper files to remove stale NUKE_ENTERPRISE_TOKEN handling#519ITaluone wants to merge 2 commits into
NUKE_ENTERPRISE_TOKEN handling#519Conversation
ebdf1a6 to
397e7ed
Compare
1d801f3 to
3e94c3f
Compare
|
@dennisdoomen Should this go into the already existing |
I would definitely make that a dedicated step. |
|
Cool :) |
b361254 to
e2cbe24
Compare
|
Should now fit definitely :) |
ITaluone
added
enhancement
e2cbe24 to
7c062b2
Compare
| return Task.CompletedTask; | ||
| } | ||
|
|
||
| internal static RewriteResult Cleanup(string content) |
There was a problem hiding this comment.
🔧 We don't want to test the implementation details of a step. Instead, we use the public API of this class.
🌱 (planting a seed) In principle testing the step directly is wrong as well. We should really be using the Migration class as the subject.
There was a problem hiding this comment.
Do I got this correct, that the one integration test would suffice then?
There was a problem hiding this comment.
I cannot see a way to not test the actual internals, without bloating this PR.
Should we for now proceed with this and refactor the tests later?
| var endOfIfBlock = lines.FindIndex(indexOfEnterpriseEnvVarCheck, | ||
| line => line.Trim() == "}" || line.Trim() == "fi"); | ||
|
|
||
| if (lines[endOfIfBlock + 1].Trim() == "") |
There was a problem hiding this comment.
🔧🤖 endOfIfBlock (line 47) can come back -1 if the if/fi/} block is malformed, and lines[endOfIfBlock + 1] here throws IndexOutOfRangeException if the block is the last content in the file. This runs against arbitrary user-modified bootstrap scripts during migration, so a hand-edited or unusual file could crash the migration step instead of degrading gracefully (e.g. skip with a warning).
There was a problem hiding this comment.
Ahm.. what does this mean?
in principle: this for example is valid bash as well
if [[ ! -z ${NUKE_ENTERPRISE_TOKEN+x} && "$NUKE_ENTERPRISE_TOKEN" != "" ]]; then
"$DOTNET_EXE" nuget remove source "nuke-enterprise" &>/dev/null || true
"$DOTNET_EXE" nuget add source "https://f.feedz.io/nuke/enterprise/nuget" --name "nuke-enterprise" --username "PAT" --password "$NUKE_ENTERPRISE_TOKEN" --store-password-in-clear-text &>/dev/null || true; fibut just grepping for "fi" is potentially more dangerous..
the +1 thing is fixed btw..
7c062b2 to
5c1845b
Compare
…PRISE_TOKEN` handling
5c1845b to
c1fa6e4
Compare
3 participants
Closes: #518
Add a small cleanup step, which removes stale env var check for the
NUKE_ENTERPRISE_TOKENvariable, which in return adds a special nuget source (which at best is stale and at worst non-existent).