Skip to content
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

False positives in skip check (after merge on top of skipped commit) #2139

Open
h-vetinari opened this issue Nov 18, 2024 · 5 comments
Open

Comments

@h-vetinari
Copy link
Member

I've run into the following kind of situation a few times now, where a merge commit gets skipped if the last commit on main contained [ci skip] etc. The following is a shortened & annotated git log --decorate --oneline --graph --all from https://github.com/conda-forge/ctng-compiler-activation-feedstock/, where

*   ddc2d49 (up/main) Merge pull request #136 from minrk/isystem-not-enough     # CI ran
|\
| * 4e19877 add -I $PREFIX/include to FFLAGS
* |   3648ba7 Merge pull request #131 from h-vetinari/clang19                   # CI skipped!
|\ \
| * | 68e8776 (origin/clang19, clang19) readd libstdcxx-devel to clangxx_impl; fix typo
| * | 111e074 [...]
| |/
* / 777282f [ci skip] [skip ci] [cf admin skip] ***NO_CI*** admin migration RemoveAutomergeAndRerender
|/
*   9833f59 Merge pull request #135 from apmorton/am/gcc-14.2                   # CI ran

In particular, the merge after conda-forge/ctng-compiler-activation-feedstock#131 seems to falsely determine that the job should be skipped. Presumably there's an issue with how skips get identified for merge commits, in that it wrongly picks up on the last non-merge commit on main (or something like that)?

CC @beckermr @jaimergp

@jaimergp
Copy link
Member

I think @beckermr had removed the skip checks in main 🤔 See #2110. It looks like that file has not been rerendered in a while, though.

@beckermr
Copy link
Member

Yes please rerender and try again.

@h-vetinari
Copy link
Member Author

That feedstock currently cannot be rerendered due to #2100. I did see it elsewhere as well, but will definitely check if a rerender solves it when I next encounter it.

@h-vetinari
Copy link
Member Author

h-vetinari commented Nov 24, 2024

Another example that wrongly gets skipped (though this case is arguably hard to avoid, because [ci skip] could presumably legitimately not be at the beginning): Revert "[ci skip] disable optional test dep sparse to unblock 3.13 migration", see here

@beckermr
Copy link
Member

Yeah and "Revert" could legitimately be part of a commit one would want to skip Ci for too. There is nothing we can do about this one unless we can get GitHub to read our minds.

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

No branches or pull requests

3 participants