Skip to content

Tests: Make tests not trigger Migrate 4.0.0-beta.1 warnings #2364

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 18, 2025
Merged

Tests: Make tests not trigger Migrate 4.0.0-beta.1 warnings #2364

merged 2 commits into from
Aug 18, 2025

Conversation

mgol
Copy link
Member

@mgol mgol commented Aug 11, 2025

Changes:

  • Checkboxradio: Change .attr( "checked", true ) to `.attr( "checked", "checked" )
  • Selectmenu: Disable the boolean-attributes patch for one assertion where it's impossible to avoid

@mgol mgol added this to the 1.14.2 milestone Aug 11, 2025
@mgol mgol requested review from fnagel and timmywil August 11, 2025 22:09
@mgol mgol self-assigned this Aug 11, 2025
@mgol mgol force-pushed the migrate-4-tests branch from eb3167f to ac886c1 Compare August 11, 2025 22:14
fnagel
fnagel previously requested changes Aug 12, 2025
View reviewed changes
Copy link
Member

@fnagel fnagel left a comment

Choose a reason for hiding this comment

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

+1 by reading, but a small typo needs to be fixed

timmywil
timmywil approved these changes Aug 12, 2025
View reviewed changes
Copy link
Member

@timmywil timmywil left a comment

Choose a reason for hiding this comment

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

LGTM, pending @fnagel's comment

mgol added 2 commits August 12, 2025 22:01
@mgol
Tests: Make tests not trigger Migrate 4.0.0-beta.1 warnings
3b6c126 
Changes:
* Checkboxradio: Change `.attr( "checked", true )` to
  `.attr( "checked", "checked" )
* Selectmenu: Disable the `boolean-attributes` patch for one assertion where
  it's impossible to avoid
@mgol
fixup! Tests: Make tests not trigger Migrate 4.0.0-beta.1 warnings
ac6bc89
@mgol mgol force-pushed the migrate-4-tests branch from ac886c1 to ac6bc89 Compare August 12, 2025 20:01
@mgol mgol requested a review from fnagel August 12, 2025 20:02
@mgol
Copy link
Member Author

mgol commented Aug 12, 2025

@fnagel PTAL

@mgol
Copy link
Member Author

mgol commented Aug 13, 2025

@fnagel BTW, if you generally approve a PR but just want a simple typo to be fixed in an obvious way then I'd prefer if you approve with a comment. I am not merging PRs blindly, I read comments first, so I'd fix the text, but then I'd be able to merge immediately, without waiting for another review.

@mgol mgol dismissed fnagel’s stale review August 18, 2025 20:12

I'll dismiss your review, @fnagel, since from your comment it seemed you only had this single remark which I fixed, Timmy approved the PR and almost a week have passed.

@mgol mgol merged commit a7ac081 into jquery:main Aug 18, 2025
11 checks passed
@mgol mgol deleted the migrate-4-tests branch August 18, 2025 20:12
@mgol mgol removed the Needs review label Aug 18, 2025
@fnagel
Copy link
Member

fnagel commented Aug 19, 2025

@fnagel BTW, if you generally approve a PR but just want a simple typo to be fixed in an obvious way then I'd prefer if you approve with a comment. I am not merging PRs blindly, I read comments first, so I'd fix the text, but then I'd be able to merge immediately, without waiting for another review.

Sure, I will do next time.

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

@timmywil timmywil timmywil approved these changes

@fnagel fnagel Awaiting requested review from fnagel

Assignees

@mgol mgol

Labels
Comp: Tests
Milestone
1.14.2
Development

Successfully merging this pull request may close these issues.
3 participants
@mgol @fnagel @timmywil