Skip to content

FIX Trigger CD build on master branch #18774

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

Merged
merged 3 commits into from
Nov 6, 2020

Conversation

alfaro96
Copy link
Member

@alfaro96 alfaro96 commented Nov 6, 2020

What does this implement/fix? Explain your changes.

This should trigger a build on push to release branches (regardless of whether the [cd build] commit trigger is present) and on PRs and push to master branch only if the [cd build] commit trigger is found.

Quick review @ogrisel @thomasjpfan?

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should trigger a build on push to release branches (regardless of whether the [cd build] commit trigger is present) and on PRs and push to master branch only if the [cd build] commit trigger is found.

I think this should be simplified further:

@ogrisel ogrisel merged commit 505f3d0 into scikit-learn:master Nov 6, 2020
@ogrisel
Copy link
Member

ogrisel commented Nov 6, 2020

Let's try this.

@alfaro96
Copy link
Member Author

alfaro96 commented Nov 6, 2020

Now, it has been triggered 😉.

Thank you @ogrisel for bearing me!

@alfaro96 alfaro96 deleted the fix_wheel_builder branch November 6, 2020 14:41
@rth
Copy link
Member

rth commented Nov 7, 2020

The additional jobs make reading CI status a bit harder. There are now 25 CI tasks on master. Another alternative is to trigger this manually (via Github UI) prior to a release on some test branch and then run it on releases (see e.g. scikit-learn-contrib/scikit-learn-extra#74).

I don't think we would care much about wheels CI, short of the few weeks before the release, and those CI statuses even if they are skipped would just create noise for most contributors.

@alfaro96
Copy link
Member Author

alfaro96 commented Nov 9, 2020

The additional jobs make reading CI status a bit harder. There are now 25 CI tasks on master. Another alternative is to trigger this manually (via Github UI) prior to a release on some test branch and then run it on releases (see e.g. scikit-learn-contrib/scikit-learn-extra#74).

I don't think we would care much about wheels CI, short of the few weeks before the release, and those CI statuses even if they are skipped would just create noise for most contributors.

+1 of using the workflow_dispatch event to trigger the Wheel builder workflow. Thus, we do not need the [cd build] commit marker and I think that we can even manually trigger the workflow in the branch of the fork (without needing to use a branch on the scikit-learn repository.

I will open a PR ASAP and we may evaluate there this proposal.

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

Successfully merging this pull request may close these issues.

3 participants