Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add perf-guard github workflow #345

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
sirbrillig merged 31 commits into 2.x from add-perf-ci-guard
Dec 9, 2024
Merged

Add perf-guard github workflow #345

sirbrillig merged 31 commits into 2.x from add-perf-ci-guard
Dec 9, 2024

Conversation

@sirbrillig
Copy link
Owner

@sirbrillig sirbrillig commented Dec 2, 2024
edited
Loading

This adds a new Github action CI check to make sure that the performance of the sniff does not jump wildly high.

It uses the PHPMailer.php file from here: https://raw.githubusercontent.com/PHPMailer/PHPMailer/refs/tags/v6.9.3/src/PHPMailer.php as a test fixture, since that file is relatively large and complex. It then sets a threshold for scanning that file with this sniff.

Fixes #343

Copy link
Owner Author

Nice, I need to pick a better target file but this does work. You can see the failure in 769bd71 when I reduced to baseline time to 0.2.

Copy link
Collaborator

jrfnl commented Dec 3, 2024

Was just thinking - I imagine that as things are now, it may be hard to debug the workflow when it fails as it may not be clear on what timing it failed.
Could be an idea to:

  • first run the report in a separate step and store the report to $GITHUB_OUTPUT.
  • In a next step to echo out the report for debug purposes
  • Then in the step after that use grep to get the timing target from the report.

Copy link
Owner Author

Good thinking! I got that to work, I believe, although I still had to collect the output, print it, and grep it in the same step because I can't figure out how to pass a multi-line github output variable through grep. I figured out how to store the multi-line output in an output variable but then echoing it back into a unix pipe seems very difficult 🤔 . Hopefully a single step will be ok for now.

@sirbrillig sirbrillig marked this pull request as ready for review December 8, 2024 19:51
Copy link
Owner Author

although I still had to collect the output, print it, and grep it in the same step because I can't figure out how to pass a multi-line github output variable through grep

I cheated and dumped the output to a file. That works.

@sirbrillig sirbrillig merged commit 6a0023c into 2.x Dec 9, 2024
64 checks passed
@sirbrillig sirbrillig deleted the add-perf-ci-guard branch December 9, 2024 01:45
Copy link
Collaborator

jrfnl commented Dec 9, 2024
edited
Loading

@sirbrillig Nicely done ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jrfnl jrfnl jrfnl left review comments

+1 more reviewer

@Synchro Synchro Synchro left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Needs performance guard CI process

AltStyle によって変換されたページ (->オリジナル) /