-
Notifications
You must be signed in to change notification settings - Fork 3.3k
{CI} Skip history check in LTS #30853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
️✔️AzureCLI-FullTest
|
|
Hi @bebound, |
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
42fa5ca to
851ebc8
Compare
851ebc8 to
e055a78
Compare
|
header -> history |
| - job: CheckHeaders | ||
| displayName: "Check License, History, and DocMap" | ||
|
|
||
| condition: and(not(contains(variables['Build.SourceBranch'], 'lts')), not(contains(variables['System.PullRequest.TargetBranch'], 'lts'))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In LTS PR:
Expanded: and(not(contains('refs/pull/30848/merge', 'lts')), not(contains('dev-lts-2.66', 'lts')))
In batched CI, System.PullRequest.TargetBranch is an empty string, and the condition is still correct.
Evaluating: and(not(contains(variables['Build.SourceBranch'], 'lts')), not(contains(variables['System.PullRequest.TargetBranch'], 'lts')))
Expanded: and(not(contains('refs/heads/lts-skip-version-check', 'lts')), not(contains(variables['System.PullRequest.TargetBranch'], 'lts')))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to consider Build.SourceBranch being refs/pull/30848/merge (PR) and System.PullRequest.TargetBranch being '' (batched CI) at all.
For PR, as contains('dev-lts-2.66', 'lts') is true, the and() fails. For batched CI, as contains('refs/heads/lts-skip-version-check', 'lts') is true, and() fails.
Maybe using or() makes the logic easier to understand:
not(or(contains(variables['Build.SourceBranch'], 'lts'), contains(variables['System.PullRequest.TargetBranch'], 'lts')))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.PullRequest.TargetBranch being '' is evaluated in dev branch batched CI.
not(contains('refs/heads/dev', 'lts') is true, and it will check not(contains('', 'lts').
Description
Like #30848, we don't have changelog for LTS and this check should be skipped.