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

Fixes incorrectly cased namespace usages. #37574

Open
wants to merge 4 commits into
base: 2.4-develop
Choose a base branch
from
Open

Fixes incorrectly cased namespace usages. #37574

wants to merge 4 commits into from

Conversation

hostep
Copy link
Contributor

@hostep hostep commented Jun 3, 2023 •
edited by m2-assistant bot

Description (*)

While playing around with phpstan on level 0 on the Magento codebase, these problems showed up (among many others, but those are for another day)

$ vendor/bin/phpstan analyse --level=0 lib | grep 'referenced with incorrect case'
 3929/3929 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

  10     Class Magento\Framework\DB\Select referenced with incorrect case: Magento\Framework\Db\Select.

$ vendor/bin/phpstan analyse --level=0 app/code/Magento | grep 'referenced with incorrect case'
 14097/14097 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

  116    Class Magento\GiftMessage\Model\Save referenced with incorrect case: Magento\Giftmessage\Model\Save.
  12     Class Magento\Sales\Model\Order\Creditmemo\Item referenced with incorrect case: Magento\Sales\Model\Order\CreditMemo\Item.
  28     Class Magento\Framework\DB\Select referenced with incorrect case: Magento\Framework\Db\Select.
  9      Class Magento\Sales\Model\Order\Creditmemo\Item referenced with incorrect case: Magento\Sales\Model\Order\CreditMemo\Item.

This PR fixes these.
I don't think these caused real issues, I think composer's autoloader is gracefully enough to load those incorrectly cased namespaces, but let's make them correct anyways.

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

  1. Run vendor/bin/phpstan analyse --level=0 {some-path} | grep 'referenced with incorrect case' on all Magento code, it should not output anything (don't run this on the root the M2 codebase, otherwise it will take hours and might crash your computer if you don't have at least 32 GB of memory)
  2. Test all affected code and make sure that everything keeps working as expected (I'm assuming the automated tests will take care of this)

Questions or comments

I might fix (static) test failures if they don't turn out to be too complex to handle, but won't write new automated tests, because I think that's not relevant in scope of this PR.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

Resolved issues:

  1. resolves [Issue] Fixes incorrectly cased namespace usages. #38640: Fixes incorrectly cased namespace usages.

@m2-assistant
Copy link

m2-assistant bot commented Jun 3, 2023

Hi @hostep. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:
  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

@hostep
Copy link
Contributor Author

hostep commented Jun 3, 2023

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@m2-community-project m2-community-project bot added this to Pending Review in Pull Requests Dashboard Jun 3, 2023
@hostep
Copy link
Contributor Author

hostep commented Jun 3, 2023

The failure of the semantic version check is in my opinion something to ignore, in practice it shouldn't cause an issue.

Pushed new commit to fix static tests.

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@hostep
Copy link
Contributor Author

hostep commented Jun 5, 2023 •
edited

Failures in functional tests don't look related at first sight (flaky tests probably)

Only the failure in the semantic version checker is related to this PR, that one will need approval I guess? (is more of a false positive)

@engcom-Hotel engcom-Hotel added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Jun 6, 2023
@m2-community-project m2-community-project bot moved this from Pending Review to Ready for Testing in Pull Requests Dashboard Jun 6, 2023
@engcom-Lima engcom-Lima self-assigned this Jun 7, 2023
@engcom-Lima
Copy link
Contributor

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@engcom-Lima engcom-Lima moved this from Ready for Testing to Testing in Progress in Pull Requests Dashboard Jun 7, 2023
@engcom-Lima engcom-Lima removed their assignment Jun 7, 2023
@engcom-Lima engcom-Lima moved this from Testing in Progress to Ready for Testing in Pull Requests Dashboard Jun 7, 2023
@engcom-Lima engcom-Lima added Progress: testing in progress Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it and removed Progress: ready for testing labels Jun 7, 2023
@m2-community-project m2-community-project bot moved this from Testing in Progress to Ready for Testing in Pull Requests Dashboard Jun 7, 2023
@sinhaparul sinhaparul added the Project: Community Picked PRs upvoted by the community label Apr 7, 2024
@m2-community-project m2-community-project bot added this to Ready for Testing in Community Dashboard Apr 7, 2024
@m2-community-project m2-community-project bot removed this from Ready for Testing in Pull Requests Dashboard Apr 7, 2024
@engcom-Charlie engcom-Charlie self-assigned this Apr 18, 2024
@engcom-Charlie engcom-Charlie moved this from Ready for Testing to Testing in Progress in Community Dashboard Apr 19, 2024
@engcom-Charlie
Copy link
Contributor

Hi @hostep,

Thank you for your contribution!

✔️ QA Passed

In order to proceed further on this PR, I have followed the mentioned steps
Before: ✖️
image

After: ✔️
image

As the build is failing, currently moving it further in Extended Testing.

Thank you!

@engcom-Charlie engcom-Charlie moved this from Testing in Progress to Extended testing (optional) in Community Dashboard Apr 19, 2024
@engcom-Charlie
Copy link
Contributor

@magento run all tests

@engcom-Charlie
Copy link
Contributor

Raised internal approval JIRA for the SVC failure. Moving this PR to Pending Approval now. We will proceed ahead on this once will get all the required approvals.

image

@engcom-Charlie engcom-Charlie moved this from Extended testing (optional) to Pending Approval in Community Dashboard Apr 19, 2024
@engcom-Charlie
Copy link
Contributor

@magento create issue

@m2-assistant m2-assistant bot mentioned this pull request Apr 19, 2024
Open
5 tasks
@m2-community-project m2-community-project bot moved this from Pending Approval to Ready for Testing in Community Dashboard Apr 19, 2024
@m2-community-project m2-community-project bot moved this from Pending Approval to Ready for Testing in Community Dashboard Apr 19, 2024
@engcom-Charlie engcom-Charlie moved this from Ready for Testing to Pending Approval in Community Dashboard Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

Assignees

@ihor-sviziev ihor-sviziev

@engcom-Charlie engcom-Charlie

Labels
Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: pending approval Project: Community Picked PRs upvoted by the community Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Community Dashboard
Pending Approval
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Issue] Fixes incorrectly cased namespace usages.
6 participants
@hostep @engcom-Lima @engcom-Charlie @ihor-sviziev @sinhaparul @engcom-Hotel