Skip to content

CI Adds skipping to azure pipelines with commit message #19134

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

Merged
merged 4 commits into from
Jan 13, 2021

Conversation

thomasjpfan
Copy link
Member

This PR adds the ability to use [ci skip] in a commit message to skip azure pipelines in pull requests.

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

I like the general idea, but I think this PR is remove the dependency on linting success for some of the jobs, but I don't think it was intentional.

Comment on lines 189 to 193
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule')
)
Copy link
Member

Choose a reason for hiding this comment

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

We should keep the dependency on linting success, no?

Suggested change
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule')
)
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule'),
succeeded('linting')
)

Comment on lines 168 to 172
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule')
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule')
)
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule'),
succeeded('linting')
)

Comment on lines 146 to 150
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule')
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule')
)
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule'),
succeeded('linting')
)

Comment on lines 98 to 102
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule')
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule')
)
condition: |
and(
not(contains(dependencies['git_commit']['outputs']['commit.message'], '[ci skip]')),
ne(variables['Build.Reason'], 'Schedule'),
succeeded('linting')
)

@thomasjpfan
Copy link
Member Author

Updated PR by adding succeeded() everywhere. This means that the job runs when the jobs fromm dependsOn was successful.

@ogrisel
Copy link
Member

ogrisel commented Jan 13, 2021

Updated PR by adding succeeded() everywhere. This means that the job runs when the jobs fromm dependsOn was successful.

Yes, I think this was the case previously.

@ogrisel ogrisel merged commit ea5342c into scikit-learn:master Jan 13, 2021
@ogrisel
Copy link
Member

ogrisel commented Jan 13, 2021

Thanks for the fix, merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants