trigger ci on pull_request

[talregev]

trigger ci on pull_request:

• The ci will stop automatically and restart on each commit in the same PR. - only the last commit will run the ci.
• The ci will trigger (and not be stopped) for each commit (push) directly to branches. - preserve the current state.

[mmuetzel]

The reasoning for the current concurrency setting is the following:
Building Octave in CI takes quite long. If multiple commits are pushed in a row during a short amount of time, we'd like the following:

• A CI run that already started should not be canceled. Instead, subsequently triggered actions should be queued. (The reasoning being: If the action might be finishing in a couple of minutes, don't waste the minutes spent so far by cancelling it now...)
• If additional changes are pushed while actions are still queued, the newer action should replace the pending action in the queue. (No need to run the CI for each and every commit.)

To be honest, I didn't attempt to understand your proposed changes to the concurrency settings (yet).
Would the behavior still be like described with your changes?

Why would we need different concurrency rules for PRs? (Assuming we'd like to enable CI on GitHub PRs.)

[talregev]

To be honest, I didn't attempt to understand your proposed changes to the concurrency settings (yet).
Would the behavior still be like described with your changes?

Yes, it preserve the current state, as it state in the second bullet.

Why would we need different concurrency rules for PRs? (Assuming we'd like to enable CI on GitHub PRs.)

I want to propose a change when doing a PR. - when you do a PR, many times you do mistakes. It not yet ready, and you want last result for your latest commit.

Then for push (regular commits) it preserve the state as you write it down.
For PR, it cancel the ci if it running, then start a new ci jobs on the latest commit.

This pattern taken from github action docs:
https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/control-the-concurrency-of-workflows-and-jobs

[mmuetzel]

Wouldn't users be able to run the action in their fork of the repository?
That also has the advantage that they have complete control of the action. They can cancel or restart the workflows however they prefer.
Why would it be better (for contributors) if the action would run for the PR?

Also, I don't think the concurrency would work like you described for pushes. Looking at the documentation you linked, github.run_id is unique to every run. So, multiple actions would run concurrently if multiple commits are pushed in a row during a short amount of time.
Is that wrong? Why wouldn't that be so?

[talregev]

Also, I don't think the concurrency would work like you described for pushes. Looking at the documentation you linked, github.run_id is unique to every run. So, multiple actions would run concurrently if multiple commits are pushed in a row during a short amount of time.

It it multiple commit in the same push, it will only trigger on the last commit.
If it have multiple push, it will have multiple ci trigger. The same it happen now. for example: this is also unique for each commit: ci-alpine-${{ github.ref }}

Wouldn't users be able to run the action in their fork of the repository?

On a self PR, only after this change. You don't commit to master or default on your forked repo. you do self PRs, to check your code.
Often you present your PR, then you asked for changes. While you do your changes, you make a mistakes.
That why I am adding the change in concurrency.

Why would it be better (for contributors) if the action would run for the PR?

For my experience working and contributing in other repos and many PRs, it more convenient and faster to develop when you see the result on each PR. You can improve very fast. With the suggestion I am suggest for concurrency in PRs, changes can be made faster, and no need to wait waisted time for unfinished jobs in your PR, because your wrong commits, or changed commits.

If you think otherwise, let me know. I will just add the pull_request without change the concurrency.

[mmuetzel]

this is also unique for each commit: ci-alpine-${{ github.ref }}

No, it isn't. It is only unique to every branch of this repository.
It does trigger for every push. But the concurrency rules prevent the action from starting until the previous action is done. That will no longer happen for your proposed changes.

On a self PR, only after this change.

I meant: Users can run the CI on the branch in their fork before or after they open a PR for said branch. They can do that now. (I do that semi-regularly.) We don't need this change for that.

If you prefer to not wait for the results when iterating, you could cancel a running action and let the subsequent one start. You can do that on your fork without needing any special permissions for this repository.
However, I often push changes for something that I noticed during a CI run while still waiting for further output from it. I'd hate if the CI canceled a job automatically just because I started pushing some follow-up change. (That might be personal preference.)

[talregev]

I meant: Users can run the CI on the branch in their fork before or after they open a PR for said branch. They can do that now. (I do that semi-regularly.)

Now I understand you what you doing.
It more convenient to see your changes in multiple commits in a self PR, because it summarize your changes, you can review yourself faster and better. Try it once. and if you also see the CI, it supper convenient that everything in one place.

I will change it back the concurrency policy.

[mmuetzel]

I'm still not convinced this is a good idea.

We are constantly running into GitHub cache limits (which makes the slow CI even slower). If we'd enable the CI on PRs, that would make the situation only worse.

GitHub is just a mirror for Octave. We need to look at multiple places anyway.

If contributors would like to run the CI before their changes are merged, they can do that in their fork of this repository, imho.

I'll leave this open for some time in case another maintainer would like to chime in. But I personally are not in favor of this change.