Invoke CI only on main branch

[yutannihilation]

If I understand correctly, the current GitHub Actions setting invokes the check on all pushes to any branches. I think an ordinary user wants to run CI only on the main branch. (Is this configured so by some intension...?)

[mlafeldt]

I think this change makes sense. On the other hand, https://github.com/duckdb/extension-template/blob/main/.github/workflows/MainDistributionPipeline.yml also covers all branches, not just main.

Maybe @samansmink or @carlopi can add more context?

[carlopi]

I think it makes more sense as is now, logic works as follow:

• _extension_distribution.yml builds and tests a given extension, and that's fine to do on PRs and on pushes (I think)
• _extension_deploy.yml actually makes the extension available (for example via a secondary repository), this step makes sense only for tags / releases / main branch

You could check for example: https://github.com/duckdb/duckdb-httpfs/blob/main/.github/workflows/MainDistributionPipeline.yml or https://github.com/duckdb/duckdb-spatial/blob/main/.github/workflows/MainDistributionPipeline.yml.

Given this repository do not handle publications of extensions but only building and testing, to me it seems fine as is now.

[yutannihilation]

• _extension_distribution.yml builds and tests a given extension, and that's fine to do on PRs and on pushes (I think)

Do you really want to invoke GitHub Actions twice when you push to the branch that a pull request is based on...?

[carlopi]

Do you really want to invoke GitHub Actions twice when you push to the branch that a pull request is based on...?

I am not sure I see what is the better proposal / if you can make a concrete example of what's not working right now.

Current setup we have for spatial or httpfs, the example I brought, seems to me works, but very open to proposals or identifying current pain point. I don't think I have enough context right now.

[yutannihilation]

Okay. Sorry, I wasn't very clear about what's the problem. Let me explain.

If a user wants to develop their own extension with pull request based development, this is what happens:

1. If a user wants to create a pull request to add a feature, they publishes the branch first. With the current setting, this invokes the CI check.
2. Then, the user creates a pull request. This also invokes the CI check. Usually, this is fine and necessary, but this time, GHA runs the same check on the same commit.
3. During polishing the pull request, every time the user pushes a new commit, it invokes two CI checks; for push and for pull_request. Again, exactly the same check on exactly the same commit.

I feel this is waste of CI time. I bet it's very rare that the results of the checks differ between on the branch and on the pull request. By "rare", I mean it's sometime useful. I might choose the same setting, but it's a trade-off between safety and CI time. Considering the CI check on DuckDB extension takes a decent amount of time, I'd choose to run the CI check only on the main branch.

Of course, it depends on the size of the development; spatial or httpfs are not the case because most of the development happens on the forked repository, which GHA is disabled in usual. But, an ordinary hobby developer like me develops on only one repository without forks.

[carlopi]

So I think you would want to add [main] to only the push event? Would that work?
That looks to achieve the goal of running CI for the following usage:

• PR (either to fork or original repo) are developed on named branches (!= `main), they are run only once due to pull_request trigger

In that case CI is run once per push (on an non-main branch, via pull_request trigger), plus once per merge (on the main branch, via push trigger).

Does this sound OK?

[yutannihilation]

Thanks, it makes sense to me! I usually add [main] condition also to pull_request, but pubably adding it to push is enough.

[samansmink]

LGTM

[yutannihilation]

Can this get merged? If there's still anything I should address, please let me know.

[carlopi]

Thanks

[yutannihilation]

Thanks for merging!