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

Type protected and private methods of legacy admin controllers #34653

Merged
merged 1 commit into from Nov 28, 2023
Merged

Type protected and private methods of legacy admin controllers #34653

merged 1 commit into from Nov 28, 2023

Conversation

Hlavtox
Copy link
Contributor

@Hlavtox Hlavtox commented Nov 22, 2023 •
edited

Questions Answers
Branch? develop
Description? Adds types to all protected and private functions of legacy admin controllers. I did not type few exceptions, mainly when the method inherited from AdminController and there was a conflict of ObjectModel vs it's child.
Type? refacto
Category? BO
BC breaks? yes
Deprecations? no
How to test?
UI Tests https://github.com/Hlavtox/ga.tests.ui.pr/actions/runs/6985011209
Fixed issue or discussion?
Related PRs
Sponsor company

@Hlavtox Hlavtox requested a review from a team as a code owner November 22, 2023 12:52
@prestonBot prestonBot added develop Branch Refactoring Type: Refactoring BC break Type: Introduces a backwards-incompatible break labels Nov 22, 2023
@Hlavtox Hlavtox force-pushed the type-legacy-admin-protected branch 3 times, most recently from 896f56b to 2c6460f Compare November 22, 2023 14:51
matthieu-rolland
matthieu-rolland approved these changes Nov 22, 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.

This is a BC break indeed :D

nice 👍

@Hlavtox Hlavtox changed the title Type all protected and private methods of legacy admin controllers Type protected and private methods of legacy admin controllers Nov 22, 2023
@Hlavtox Hlavtox added the Needs documentation Needs an update of the developer documentation label Nov 22, 2023
@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Nov 25, 2023
@Hlavtox
Copy link
Contributor Author

Hlavtox commented Nov 27, 2023

@florine2623 @PrestaShop/qa-automation

The test is failing, but I think because the element is under the scrollbar. :D The feature is working normally. :-)

When I try it
Snímek obrazovky 2023-11-27 115638

Auto test artifact
fail_test_1

@nesrineabdmouleh
Copy link
Contributor

nesrineabdmouleh commented Nov 27, 2023 •
edited

Hello @Hlavtox,
Yes the feature is working normally, the error is related to the selector of the error message => PR to fix

@Hlavtox
Copy link
Contributor Author

Hlavtox commented Nov 27, 2023

So @florine2623 I think it's QA ✅? :-)

@florine2623 florine2623 added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Nov 28, 2023
@prestonBot
Copy link
Collaborator

QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge.

@Hlavtox Hlavtox merged commit 28602bf into PrestaShop:develop Nov 28, 2023
18 checks passed
@Hlavtox Hlavtox added this to the 9.0.0 milestone Nov 28, 2023
@Hlavtox Hlavtox deleted the type-legacy-admin-protected branch January 24, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FabienPapet FabienPapet FabienPapet approved these changes

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

@matks matks matks approved these changes

@tleon tleon tleon approved these changes

@ShaiMagal ShaiMagal ShaiMagal approved these changes

@boherm boherm boherm approved these changes

Assignees
No one assigned
Labels
BC break Type: Introduces a backwards-incompatible break develop Branch 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