-
Notifications
You must be signed in to change notification settings - Fork 534
Faster code downgrade to speedup CI #4352
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
Conversation
.github/workflows/lint.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd do this a bit differently:
- Install
${{ matrix.php-version }} - Run the composer steps
- Only when downgrading: install 8.4
- Transform source code
- Only when downgrading: install
${{ matrix.php-version }}again
You can try playing with https://docs.github.com/en/actions/tutorials/create-actions/create-a-composite-action to see if we can achieve DRY across different workflows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I want to avoid is always start by installing 8.4 which is immediately going to be ovewritten with matrix version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd do this a bit differently:
done
You can try playing with docs.github.com/en/actions/tutorials/create-actions/create-a-composite-action to see if we can achieve DRY across different workflows.
I would do that in a followup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what todo with the "Compile PHAR" - or what the actual problem is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you link the failure? I don't see anything big.
If you mean "Checksum PHAR" - that's fine. The idea behind that job is that the PHAR checksum should not change when src/ doesn't change.
If we change something about the compilation process then it's fine if PHAR contents change. I suspect that some ConstFetches now use FQN like \null so the source slightly changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean "Checksum PHAR" - that's fine.
yep, that one. wasn't sure what it means
This pull request has been marked as ready for review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No if condition here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
.github/workflows/lint.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you link the failure? I don't see anything big.
If you mean "Checksum PHAR" - that's fine. The idea behind that job is that the PHAR checksum should not change when src/ doesn't change.
If we change something about the compilation process then it's fine if PHAR contents change. I suspect that some ConstFetches now use FQN like \null so the source slightly changed.
7776279
into
phpstan:2.1.x
Really great! 🎉
Uh oh!
There was an error while loading. Please reload this page.
make use of ondrejmirtes/simple-downgrader#14
downgrading phpstan-src in CI before this PR took ~10min (windows) and ~4m30s (ubuntu)
we are now down to ~15-25 seconds