8000 Remove redundant methods by Usik2203 · Pull Request #29147 · magento/magento2 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove redundant methods #29147

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
Open
wants to merge 18 commits into
base: 2.4-develop
Choose a base branch
from
Open

Remove redundant methods #29147

wants to merge 18 commits into from

Conversation

Usik2203
Copy link
Contributor
@Usik2203 Usik2203 commented Jul 15, 2020 •
edited by m2-assistant bot
Loading

This PR removes redundant methods
_isAllowed() in app/code/Magento/AsynchronousOperations/Controller/Adminhtml/Index/Index.php
_toHtml() in app/code/Magento/Sales/Block/Adminhtml/Order/Create/Newsletter.php
These methods just call parents methods

Resolved issues:

  1. resolves [Issue] Remove redundant methods #29748: Remove redundant methods

@magento-engcom-team magento-engcom-team added Component: AsynchronousOperations Component: Sales Release Line: 2.4 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Jul 15, 2020
@Usik2203
Copy link
Contributor Author

@magento run all tests

@Usik2203
Copy link
Contributor Author

@magento run all tests

lbajsarowicz
lbajsarowicz approved these changes Jul 15, 2020
View reviewed changes
app/code/Magento/AsynchronousOperations/Controller/Adminhtml/Index/Index.php Outdated
@@ -27,32 +32,25 @@ class Index extends \Magento\Backend\App\Action

/**
* Details constructor.
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

app/code/Magento/Sales/Block/Adminhtml/Order/Create/Newsletter.php Outdated
@@ -12,7 +12,7 @@
* @author Magento Core Team <core@magentocommerce.com>
* @since 100.0.2
*/
class Newsletter extends \Magento\Sales\Block\Adminhtml\Order\Create\AbstractCreate
class Newsletter extends AbstractCreate
{
/**
* Constructor
Copy link
Contributor

Choose a reason for hiding this comment

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

By the way you can remove this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ghost ghost assigned lbajsarowicz Jul 15, 2020
@magento-engcom-team
Copy link
Contributor

Hi @lbajsarowicz, thank you for the review.
ENGCOM-7837 has been created to process this Pull Request
✳️ @lbajsarowicz, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@Usik2203
Copy link
Contributor Author

@magento run all tests

@engcom-Alfa engcom-Alfa self-assigned this Jul 17, 2020
@engcom-Alfa
Copy link
Contributor
engcom-Alfa commented Jul 17, 2020 •
edited
Loading

@lbajsarowicz Could you put an appropriate label for test coverage?
Thanks!

@lbajsarowicz lbajsarowicz added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Jul 17, 2020
@lbajsarowicz
Copy link
Contributor

@engcom-Alfa Done.

@engcom-Kilo engcom-Kilo self-assigned this Aug 7, 2020
@VladimirZaets VladimirZaets added Priority: P4 No current plan to fix. Fixing can be deferred as a logical part of more important work. Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”. labels Aug 11, 2020
@engcom-Kilo engcom-Kilo removed their assignment Aug 25, 2020
@ct-prd-projects-boards-automation ct-prd-projects-boards-automation bot moved this from Ready for Testing to Review in Progress in Community Dashboard Apr 25, 2025
@ct-prd-projects-boards-automation ct-prd-projects-boards-automation bot moved this from Review in Progress to Ready for Testing in Community Dashboard Apr 25, 2025
@engcom-Charlie
Copy link
Contributor

Manual testing is not required here, as this PR is doing cleanup activity. Currently the build is failing hence moving it to Extended Testing.

@engcom-Charlie engcom-Charlie moved this from Ready for Testing to Testing in Progress in Community Dashboard Apr 28, 2025
@engcom-Charlie engcom-Charlie moved this from Testing in Progress to Extended testing (optional) in Community Dashboard Apr 28, 2025
@engcom-Charlie
Copy link
Contributor

@magento run all tests

@engcom-Charlie
Copy link
Contributor

@magento run Functional B2B, Functional CE, WebAPI Tests

@engcom-Charlie
Copy link
Contributor

@magento run all tests

@engcom-Charlie
Copy link
Contributor

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE, Integration Tests, WebAPI Tests

@engcom-Charlie
Copy link
Contributor

The approval JIRA for SVC failure has been approved as mentioned in above comment.

The Functional B2B failure is not consistent in recent 2 builds, its neither related to PR nor failing because of PR changes. It seems to be flaky. Hence moving the PR to Merge in Progress.

Run 1:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/29147/435c7bd985f3bb2a29c8a099ef6cd2ea/Functional/allure-report-b2b/index.html#categories/b6fa66c5f98857258a3e7e1521cbe4c7/cf08c8d10dd0eded/

image

Run 2:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/29147/21d6574fb8fb921a044a1f818f0b0181/Functional/allure-report-b2b/index.html#categories

image

@engcom-Charlie engcom-Charlie moved this from Extended testing (optional) to Merge in Progress in Community Dashboard Apr 29, 2025
@engcom-Charlie
Copy link
Contributor

We are getting conflicts into this PR. To look into it moving it to Extended Testing.

@engcom-Charlie engcom-Charlie moved this from Merge in Progress to Extended testing (optional) in Community Dashboard May 19, 2025
@engcom-Charlie
Copy link
Contributor

@magento run all tests

@engcom-Charlie
Copy link
Contributor

@magento run Functional Tests B2B

@engcom-Charlie
Copy link
Contributor

The approval JIRA for SVC failure has been approved as mentioned in above comment.

One of the Functional B2B failure is not consistent in recent 2 builds, its neither related to PR nor failing because of PR changes. It seems to be flaky. The remaining 2 are consistent failures and known issues. Hence moving the PR to Merge in Progress.

Run 1:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/29147/c90d8dea979427850e1bcbb4665b8205/Functional/allure-report-b2b/index.html#categories/f22a8bb7611a88a2b18c9ea37c28679d/ea60a6a6df4bf00f/

image

Run 2:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/29147/785ae7f88dd4f83a9a78f96644585d16/Functional/allure-report-b2b/index.html#categories/8f0831c1d06562890f5ec21062dae309/e074755cf66bd311/

image

Known Issues:
ACQE-7956: StorefrontQuoteRenameReasonInputValidationTest
ACQE-7971: StoreFrontSimpleProductWithSpecialAndTierDiscountPriceTest

@engcom-Charlie engcom-Charlie moved this from Extended testing (optional) to Merge in Progress in Community Dashboard May 21, 2025
@engcom-Charlie
Copy link
Contributor

Moving this PR to Extended Testing to remove the conflicts.

@engcom-Charlie engcom-Charlie moved this from Merge in Progress to Extended testing (optional) in Community Dashboard Jun 24, 2025
@engcom-Charlie
Copy link
Contributor

@magento run all tests

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

@engcom-Charlie engcom-Charlie engcom-Charlie left review comments

@Den4ik Den4ik Den4ik approved these changes

@lbajsarowicz lbajsarowicz lbajsarowicz approved these changes

@ihor-sviziev ihor-sviziev ihor-sviziev approved these changes

@engcom-Hotel engcom-Hotel engcom-Hotel approved these changes

Assignees

@Den4ik Den4ik

@lbajsarowicz lbajsarowicz

@ihor-sviziev ihor-sviziev

@engcom-Alfa engcom-Alfa

@engcom-Charlie engcom-Charlie

@engcom-Foxtrot engcom-Foxtrot

@engcom-Hotel engcom-Hotel

Labels
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests Cleanup Component: AsynchronousOperations Component: Sales Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner Priority: P4 No current plan to fix. Fixing can be deferred as a logical part of more important work. Progress: ready for testing Project: Community Picked PRs upvoted by the community Release Line: 2.4 Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Community Dashboard
Status: Extended testing (optional)
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Issue] Remove redundant methods
0