To make backports between 11.x and 10.x easier (see #3239139: Refactor (if feasible) uses of the jQuery animate function to use Vanilla/native), we can use assertLessThan() instead of assertSame() for CSS/JS assert sizes in performance tests.
If we set appropriate assertions for both 11.x and 10.3.x, that will save having to update the performance tests on every small change to CSS or JavaScript, while still requiring new library additions to increase the budget, and hopefully still pick up any aggregation bugs.
Actual performance/size improvements should explicitly lower the budgets on both branches.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3441844-nr-bot.txt | 90 bytes | needs-review-queue-bot |
| #11 | 3441844-nr-bot.txt | 90 bytes | needs-review-queue-bot |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !7600 / changes, plain diff MR !7593changes, plain diff MR !7584
Comments
Comment #2
catchMade a start - just did round numbers c500bytes bigger than the current assertions. That should allow for refactorings of existing files, but not entire new files to be added without making a change. If it's still awkward, we could increase them in another issue.
Comment #4
nod_seems reasonable to me, and when we get over it we increase by 500b or something like that?
Comment #5
nod_will need a 10.3.x MR since it's likely it won't cherry pick :D
Comment #6
alexpottCommitted d4fad8d and pushed to 11.x. Thanks!
Moving back to needs work for the 10.3.x version
Comment #8
longwave(lol, crosspost)
Yeah this doesn't cherry-pick cleanly, leaving open for backport to 10.3.x.
Committed 05e4d1a and pushed to 11.x. Thanks!
Comment #10
catch10.3 MR is up.
Comment #11
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #13
catchHiding the 11.x MR to see if that makes needs review bot happier.
Comment #14
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #15
nod_looks good to me, bot is just getting in the way. it's not too smart to change the issue major version fast
Comment #16
alexpottCommitted d250958 and pushed to 10.3.x. Thanks!
I'm not sure I agree with that. I think the needs review bot should work better.
Comment #18
nod_I agree, not much time for it though
Comment #20
quietone commentedLovely to see this change. Thanks everyone.
Comment #23
dipakmdhrm commentedReopening MR for 10.3.x for budgeting in more places. This is needed to avoid failures for changes such as the ones in this issue: https://www.drupal.org/project/drupal/issues/3439580
These changes where there for the MR for 11.x, but not for the one for 10.3.x.
Comment #25
catchYes those were just missed. Committed/pushed to 10.3.x, thanks!