Skip to content

Fix string parsing bug#505

Merged
jonfortescue merged 13 commits intomasterfrom
TheWorstThing
Jul 16, 2019
Merged

Fix string parsing bug#505
jonfortescue merged 13 commits intomasterfrom
TheWorstThing

Conversation

@jonfortescue
Copy link
Copy Markdown
Contributor

@riarenas enjoy

@jonfortescue jonfortescue requested a review from a user July 2, 2019 21:40
@jonfortescue jonfortescue self-assigned this Jul 2, 2019
Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The azdoflow-nonbatched-all-checks-successful.ps1 test is still failing with the same symptoms.

  "policy": {
    "batchable": false,
    "updateFrequency": "none",
    "mergePolicies": [
      {
        "name": "AllChecksSuccessful",
        "properties": {
          "ignoreChecks": [
            "'Work",
            "item",
            "linking'"
          ]
        }
      }
    ]
  }

Co-Authored-By: Ricardo Arenas <riarenas@microsoft.com>
Push-Location -Path $(Get-Repo-Location $targetRepoName)
Darc-Command add-dependency --name 'Foo' --type product --repo $sourceRepoUri
Darc-Command add-dependency --name 'Bar' --type product --repo $sourceRepoUri
Darc-Command @( "add-dependency", "--name", "Foo", "--type product", "--repo", "$sourceRepoUri" )
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Darc-Command @( "add-dependency", "--name", "Foo", "--type product", "--repo", "$sourceRepoUri" )
Darc-Command @( "add-dependency", "--name", "Foo", "--type", "product", "--repo", "$sourceRepoUri" )

All the github tests are failing when adding dependencies due to a similar bug. Makes me think this approach is a little brittle and cumbersome to write? Not sure if I have a better suggestion yet though.

Write-Host ""
Write-Host "Setting repository merge policy to standard"
Darc-Set-Repository-Policies $repoUri $branchName "--standard-automerge"
Darc-Set-Repository-Policies -repo $repoUri -branch $branchName -policyParams @( "--standard-automerge" )
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test failed with:

https://github.com/maestro-auth-test/maestro-test1 @ 104674302 - Merge Policies: []
Repository merge policies should be standard, but were not

So the policy params are not getting interpreted correctly.

@ghost
Copy link
Copy Markdown

ghost commented Jul 9, 2019

Only 8/13 tests are passing when running the tests in this branch against INT.

Failed tests:
azdoflow-batched
githubflow-batched.ps1
githubflow-nonbatched-all-checks-successful.ps1
githubflow-nonbatched-with-coherency.ps1
repo-policies.ps1

@jcagme
Copy link
Copy Markdown
Contributor

jcagme commented Jul 9, 2019

Can we make sure to test this if we are not currently doing so? Discovering that test fail until we try to deploy to is not super efficient.

@jonfortescue
Copy link
Copy Markdown
Contributor Author

@jcagme yeah, we're testing it now. For whatever reason, it seemed to have passed the smoke test previously before deployment, so I most likely just missed something then.

@jonfortescue jonfortescue requested a review from a user July 10, 2019 23:31
Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With some quote trimming, I really like where this ended up. Thanks for this @jonfortescue

@jonfortescue jonfortescue merged commit c70f494 into master Jul 16, 2019
@jonfortescue jonfortescue deleted the TheWorstThing branch July 16, 2019 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants