[ENH] ensure update-contributors workflow does not run on main

[fkiraly]

(good first issue for contrinbutors with GHA experience)

The update-contributors workflow (which compiles the all-contributorsrc list into the CONTRIBUTORS page) is triggered on any branch, including main.

On main, however, it fails due to credentials, and anyway nothing should directly modify main.

An attempt to change the trigger condition failed: #6133, this seems due to trying to push to forks, as @yarnabrina has noted and @Xinyu-Wu-0000 explains here: #6093 (comment)

It would be great if we could modify the workflow so:

• it does not run on main
• it does not invalidate results of the normal CI which are no longer displayed in the GitHub UI. Optimally, the "update contributors" worflow (or a job) should run before, not after running normal CI.

[fkiraly]

Pinging @MEMEO-PRO and @Xinyu-Wu-0000 due to earlier comments or contributions to GHA.

[duydl]

Possible solutions:

it does not run on main

Add

push:
    branches-ignore:
      - main

"update contributors" worflow (or a job) should run before, not after running normal CI.

Add needs: [Update Contributors] at the yml file starting normal CI.

I used sematic-release before and gained some familiarity with GHA. Just commented from vague memory though, not enough time to confirm and make a PR.

[Greyisheep]

@fkiraly

Please, check linked PR, and give feedback

[sammychinedu2ky]

• it does not invalidate results of the normal CI which are no longer displayed in the GitHub UI. Optimally, the "update contributors" worflow (or a job) should run before, not after running normal CI.

@fkiraly please what do you mean by it does not invalidate results of the normal CI. Could you explain why it should be executed before the test workflow?

[fkiraly]

reopening in case there is still need for discussion.

[fkiraly]

@sammychinedu2ky, adding my explanation from #6189 here:

Currently (prior to #6189), update-contributors and test lead to different workflow runs, e.g., here:

It seems that the test workflow triggers first, so the GitHub UI wil only register the latest workflow run for checking whether tests have passed on the PR, to validate whether required jobs have passed.

That is, while things like test-nosoftdeps run on the test action, the outcome becomes irrelevant for the purpose of informing the Github UI, because it's looking for the job in the update-contributors workflow.

Consequently, the jobs dashboard on the pull request looks like this:

even if all the jobs are run (but in the second-to-last workflow call form the PR), and no merge button appears.

If the update-contributors workflow were to run first, and then test, this would not be a problem - but it is unclear to me how to ensure that.

[sammychinedu2ky]

So currently a change push to the contribution file will also trigger the test workflow
So will this help resolve this if this change is added to the test workflow @fkiraly

on:
  push:
    branches:
      - main
  workflow_run:
    workflows: ["update_contribution"]
    types:
      - completed
     

[fkiraly]

Thanks for the suggestion. Since I'm not that expert with GHA, could you describe in plain English what the intended behaviour is, of the code you posted?

[Greyisheep]

So currently a change push to the contribution file will also trigger the test workflow

So will this help resolve this if this change is added to the test workflow @fkiraly

on:

  push:

    branches:

      - main

  workflow_run:

    workflows: ["update_contribution"]

    types:

      - completed

     

This was my addition to the "normal CI" @fkiraly, I think this will be a good fix, from my understanding of the problem

[fkiraly]

I mean, can you please describe in plain English what your proposed change is intending to do, under which condition what happens, etc?

[sammychinedu2ky]

So currently a change push to the contribution file will also trigger the test workflow So will this help resolve this if this change is added to the test workflow @fkiraly

on:
  push:
    branches:
      - main
  workflow_run:
    workflows: ["update_contribution"]
    types:
      - completed
     

So this tells the runner to run the test flow either during a push when the update_contributor flow isn't running or to be triggered when the update_contributor flow runs but after it has completed it's execution.

[yarnabrina]

Questions for @Greyisheep and @sammychinedu2ky

1. What happens when there is no contribution update? Does it count as "condition satisfied" for test.yml or not? I hope it will still be run considering different trigger case as union sense.
2. If this workflow runs for a fork branch, I think it fails quite often (not certain on this part, may be @fkiraly can give details). So does this mean unit tests are going to be skipped always as the criterion of successful contribution update will not be fulfilled?

General question: is there a way to ensure that update contributor workflow only runs in a PR of its own instead of modifying apparently random PR's (even those that don't modify all contributors file)?

[sammychinedu2ky]

I think it will still run since a completed state is different from a successful state which you can see here
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-based-on-the-conclusion-of-another-workflow

For your general question, I think the update_contributor workflow only runs in a pr of its own when the contributor file is edited @yarnabrina

[yarnabrina]

For your general question, I think the update_contributor workflow only runs in a pr of its own when the contributor file is edited

This is not the case. Here are some examples:

#5887
#6051

it will still run since a completed state is different from a successful state

I couldn't find where it explained what all are considered as completed state. Can you please explain in plain English? Sorry for making this request repeatedly, we are not GHA experts.

[sammychinedu2ky]

Hi @yarnabrina
The conclusion is different from the activity type.
So basically you have three activity types that you can set:

• completed
• requested
• in_progress
  In this configuration,

on:
  push:
    branches:
      - main
  workflow_run:
    workflows: ["update_contribution"]
    types:
      - completed
     

the activity type in the sample above is set to completed, Which means run the test flow once the update contribution flow is complete or when a push event to the main branch is triggered.. now you can go further to run a job in the test flow depending on the conclusion of the activity. Which could be a failure or a success. You can find that here:
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-a-workflow-based-on-the-conclusion-of-another-workflow

[yarnabrina]

I'd wait for other @sktime/core-developers to share their opinion, but I'd prefer myself to run this workflow independently of unit tests in a complete different PR.

[sammychinedu2ky]

I'd wait for other @sktime/core-developers to share their opinion, but I'd prefer myself to run this workflow independently of unit tests in a complete different PR.

Yh will be better to test independently to be sure 💯