Skip to content
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

Upgrade jquery to latest version, drop polyfills #34382

Merged
merged 1 commit into from Nov 2, 2023
Merged

Upgrade jquery to latest version, drop polyfills #34382

merged 1 commit into from Nov 2, 2023

Conversation

Hlavtox
Copy link
Contributor

@Hlavtox Hlavtox commented Oct 25, 2023 •
edited by matthieu-rolland

Questions Answers
Branch? develop
Description? Upgrades jQuery to 3.7.1 everywhere, drops live polyfills, update jquery.validate to latest version, hotffixed deprecations in some dead plugins.
Type? refacto
Category? CO
BC breaks? yes
Deprecations? yes
How to test? Tests green.
UI Tests https://github.com/Hlavtox/ga.tests.ui.pr/actions/runs/6651799920
Fixed issue or discussion?
Related PRs
Sponsor company

➡️ Keeping migrate scripts for now, as we would need to upgrade jQueryUI. I will do it in a separate PR.

@Hlavtox Hlavtox requested a review from a team as a code owner October 25, 2023 12:14
@prestonBot prestonBot added develop Branch Refactoring Type: Refactoring BC break Type: Introduces a backwards-incompatible break labels Oct 25, 2023
@PrestaShop PrestaShop deleted a comment from prestonBot Oct 25, 2023
mflasquin
mflasquin previously approved these changes Oct 25, 2023
View reviewed changes
nicosomb
nicosomb previously approved these changes Oct 25, 2023
View reviewed changes
Copy link
Contributor

@nicosomb nicosomb left a comment

Choose a reason for hiding this comment

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

🎉

@kpodemski kpodemski added Key feature Notable feature to be highlighted Needs documentation Needs an update of the developer documentation labels Oct 26, 2023
@Hlavtox Hlavtox dismissed stale reviews from nicosomb and mflasquin via 91189d8 October 26, 2023 08:00
@Hlavtox
Copy link
Contributor Author

Hlavtox commented Oct 26, 2023

Updated jquery validate plugin, now the tests should be good, hopefully

@Hlavtox
Copy link
Contributor Author

Hlavtox commented Oct 26, 2023

@nicosomb @mflasquin @kpodemski All good, tests green. :-) https://github.com/Hlavtox/ga.tests.ui.pr/actions/runs/6651799920

Last step will be to remove jquery migrate, but for that, we need to upgrade jQuery UI, which is a bit of a problem. They don't provide built packages with separated files we need, so it will need some more thought.

matthieu-rolland
matthieu-rolland reviewed Oct 26, 2023
View reviewed changes
Copy link
Contributor

@matthieu-rolland matthieu-rolland left a comment

Choose a reason for hiding this comment

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

nice !

what are the removed image files about ?

@Hlavtox
Copy link
Contributor Author

Hlavtox commented Oct 26, 2023

@matthieu-rolland Unused jquery ui theme, it's using the base one. :)

matthieu-rolland
matthieu-rolland approved these changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@matthieu-rolland matthieu-rolland left a comment

Choose a reason for hiding this comment

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

LGTM, and ui tests are fine too, nice work 👍

@M0rgan01 M0rgan01 added this to the 9.0.0 milestone Oct 31, 2023
M0rgan01
M0rgan01 approved these changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@M0rgan01 M0rgan01 left a comment

Choose a reason for hiding this comment

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

Nice work !

@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Oct 31, 2023
@florine2623
Copy link
Contributor

As discussed with the team, this doesn't need to be tested by QA as auto tests is doing the job

@florine2623 florine2623 added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Nov 2, 2023
@nicosomb nicosomb merged commit 08532e9 into PrestaShop:develop Nov 2, 2023
18 checks passed
@nicosomb
Copy link
Contributor

nicosomb commented Nov 2, 2023

Thank you @Hlavtox !

@Hlavtox Hlavtox deleted the upgrade-jquery branch November 10, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthieu-rolland matthieu-rolland matthieu-rolland approved these changes

@M0rgan01 M0rgan01 M0rgan01 approved these changes

@nicosomb nicosomb nicosomb left review comments

@mflasquin mflasquin mflasquin left review comments

Assignees
No one assigned
Labels
BC break Type: Introduces a backwards-incompatible break develop Branch Key feature Notable feature to be highlighted Needs documentation Needs an update of the developer documentation QA ✔️ Status: check done, code approved Refactoring Type: Refactoring
Projects
PR Dashboard
Archived in project
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@Hlavtox @florine2623 @nicosomb @matthieu-rolland @mflasquin @M0rgan01 @kpodemski @ps-jarvis @prestonBot