Skip to content

Move nightly to workflow_call #16642

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

Closed
wants to merge 1 commit into from
Closed

Conversation

iluuu1994
Copy link
Member

@iluuu1994 iluuu1994 commented Oct 29, 2024

This PR removes most of the logic of nightly_matrix.php by converting nightly.yml to a workflow_call, and invoking it multiple times for each branch in a new root.yml workflow. The naming of the files is intentionally kept to make the diff readable. They may still be renamed afterward.

A test run is here. I'll check tomorrow whether it actually works. 🙂
Edit: Another one

  • Coverage and Pecl should only run on master
  • libmysql produces a compile error on 8.1
  • Observer tests are missing the zend_test.observer.show_output=1 ini setting on 8.1

All errors, except for the community build that also occurs in the current nightly should be fixed. I will look at that soon.

@iluuu1994 iluuu1994 marked this pull request as ready for review October 30, 2024 13:52
@iluuu1994 iluuu1994 requested a review from TimWolla as a code owner October 30, 2024 13:52
@iluuu1994 iluuu1994 requested a review from cmb69 October 30, 2024 13:52
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you very much! This looks like a very nice improvement; especially the job overview (on the left) is now readable. I assume that this also supersedes #16144.

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

Didn't look at all the changes in detail, but the results are clearly much better. Thanks!

@iluuu1994
Copy link
Member Author

I assume that this also supersedes #16144.

I forgot about this one. I think that PR still makes sense.

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