Update ci skip documentation in infrastructure.md
PR Checklist:
- [ ] note any issues closed by this PR with closing keywords
- [ ] if you are adding a new page under
docs/orcommunity/, you have added it to the sidebar in the corresponding_sidebar.jsonfile - [ ] put any other relevant information below
I noticed that when you use the bot to do non-packaging changes like update maintainer it only uses [ci skip]. So is it safe for the documentation to suggest only [ci skip] now?
Also, are the linked issues still relevant? https://github.com/conda-forge/conda-forge.github.io/issues/629 does not seem useful to me. It talks about some nuances with Travis, Circle, and Azure, but I am not sure what to take away as actionable. All my feedstocks only use Azure now. I think https://github.com/conda-forge/staged-recipes/issues/1148 is similarly linked for nuances between CI systems.
Also, I think #498 can be closed since skipping CI is already in the docs.
Deploy Preview for conda-forge-previews ready!
| Name | Link |
|---|---|
| Latest commit | 31611d858899166c95b6c86a82d9d7231f6e5c31 |
| Latest deploy log | https://app.netlify.com/sites/conda-forge-previews/deploys/66e58af049874400085da389 |
| Deploy Preview | https://deploy-preview-2297--conda-forge-previews.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
There have been several iterations in Azure (see https://github.com/Microsoft/azure-pipelines-agent/issues/1270), so we had to combine different "skipper" formulas to make sure we were not triggering CI services in large migrations (e.g. admin-migrations).
So it's one of those things that might not hurt to keep around "just in case".
Edit: Yes, some of those issues can be closed. There's some info worth keeping in the docs though. Can you add some text about this comment?
Hmm, https://github.com/microsoft/azure-pipelines-agent/issues/1270 is interesting. It highlights that the skipper phrases only work with Azure Pipelines for merge commits, not for PRs. For PRs, the CI is still triggered. Here is a recent PR from the bot that uses [skip ci]: https://github.com/conda-forge/adversarial-robustness-toolbox-feedstock/pull/50. We can see that Azure Pipelines still runs on https://github.com/conda-forge/adversarial-robustness-toolbox-feedstock/pull/50/commits/3a19536add1e5dda1fa27d18fc3c1af8efc0141c while the merge commit https://github.com/conda-forge/adversarial-robustness-toolbox-feedstock/commit/0be9377b31d76a5720917432673252f3ef125611 does not. The GitHub Actions cf linter does not run on the PR though. As a test, I opened a PR using ***NO_CI*** in the title and commit message in https://github.com/conda-forge/ibm-platform-services-feedstock/pull/63. In this case, both the Azure Pipelines and GitHub Actions jobs ran. This agrees with https://github.com/conda-forge/staged-recipes/issues/1148#issuecomment-817125955 that you linked to.
So I don't see a case where ***NO_CI*** does something that [skip ci] does not do. I will leave it though if you think that is best. If we don't remove the ***NO_CI*** suggestion, we can change this PR to mention that the phrase only works on merge commits and not PRs and remove the old links. Is that what you would prefer?
I am not sure if CI running on PRs is what conda-forge wants or just a consequence of Azure Pipelines' design decision. conda-forge could add its own rule into its Azure Pipelines templates to skip PRs to save some CI time like numpy did (https://github.com/numpy/numpy/pull/21879#issuecomment-1217908810).
The GitHub Actions cf linter does not run on the PR though.
Uhh yea, that's not great. @beckermr, what do you think about this? Does it interact with the "mergeability" checks?
I am not sure if CI running on PRs is what conda-forge wants or just a consequence of Azure Pipelines' design decision. conda-forge could add its own rule into its Azure Pipelines templates to skip PRs to save some CI time like numpy did (numpy/numpy#21879 (comment)).
I think it's mostly an AZP limitation. This logic you linked to could be added to the conda-smithy templates to mitigate this surprising behaviour, but for sure we should document this clearly now so it's not buried in an issue somewhere.
I believe this is the intended behavior for the linter. If it sees [ci skip] it skips ci.
Thanks @wshanks, ended up adding https://github.com/conda-forge/conda-smithy/pull/2077 and https://github.com/conda-forge/staged-recipes/pull/27765 for Azure via your suggestion.
Originally [ci skip] and [skip ci] worked for all CI providers (though I think Travis only liked one of those and forget what it was) and we added it to the linter
As we have moved from the original setup of Travis, CircleCI and AppVeyor through various additions and removals we have reached the current state of Azure, Travis and GitHub Actions (leveraged directly or using some other self-hosted runner solution)
Have forgotten some of the nuances of the different systems in between and how they handled skip notes. That said, [ci skip] and [skip ci] are pretty standard on most CI providers. So we have tried to make sure that works somehow. Thought Azure did some work on supporting this somewhere, but it has been a while since I looked and there are probably caveats
Since sometimes CI providers have not consistently done what we have wanted. For a time we added logic to fast cancel jobs in certain conditions (not the latest build for a PR or commit, recipe has a skip: true condition, PR doesn't merge cleanly into base branch, etc.) as well as test on merge commits consistently. Think as systems changed we dropped that logic. We could bring something like that back if it would improve user experience
Thanks @jakirkham. We just merged a pr to fast finish prs with skip messages for azure in smithy. To be honest it seems like we should just keep the logic around no matter what given the turnover over the years.