-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Perf Tests: Update base point to compare #54111
Conversation
@@ -66,13 +66,13 @@ jobs: | |||
- name: Compare performance with base branch | |||
if: github.event_name == 'push' | |||
# The base hash used here need to be a commit that is compatible with the current WP version | |||
# The current one is 34af5829ac9edb31833167ff6a3b51bea982999c and it needs to be updated every WP major release. | |||
# The current one is bd2a881101727b03b0be09382b34841af5a3c03e and it needs to be updated every WP major release. |
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.
how'd we come up with this commit SHA?
Block Supports: Change prefix in gutenberg_apply_colors_support to wp_ in dynamic blocks (#51989)
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.
nevermind I see the answer in the PR description. though I'm still curious: is that WP beta1? GB beta 1?
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.
WP 6.3 beta 1. We don’t do beta releases for the plugin.
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.
This shouldn't break anything, and it looks proper. I'm curious what led you to pick the commit SHA you did; how would we know what to pick in the future?
@dmsnell, to be honest it’s was “random” pick. I believe we can’t use SHA from the release branch as it breaks codevitals (See #51689) and can’t pick more recent backport SHAs since there will be changes not included in WP 6.3. As I mentioned in then issue we need to document this step. My current change is based on previous PRs. |
What?
Resolves #54107.
PR updates the base commit for performance tests.
Why?
The commit needs to be compatible with the current WP version.
How?
I checked previous PRs for the same task (#51381, #51689) and used my best judgment:
Testing Instructions
I think we need to merge this to test on the trunk properly.