-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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.
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
Remove deprecated HttpHeadActionInterface class #35201
base: 2.4-develop
Are you sure you want to change the base?
Conversation
Hi @markshust. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
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 Magento Contributor Guide documentation. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket. ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
Hi @markshust, |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
@ihor-sviziev understood. How can we view the details of the failed tests? I'm getting timeouts when trying to check them out, so I'm not sure what is failing. |
Hey @markshust I assume failing is not related to changes |
@markshust @ihor-sviziev @nuzil |
@@ -1,21 +0,0 @@ | |||
<?php | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markshust Can we please have this file back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manavluhar @nuzil the whole point of this ticket was to remove this file, as it is deprecated, only used in one spot, and shouldn't be causing failing tests. I'm unsure why someone would want to keep this file around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markshust backward compatibility, @nuzil can add further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that's what SemVer and the patch releases are for. How long will cruft like this build up if we never have plans to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got your point @markshust, let the maintainer review it.
Hi @markshust Thanks for your contribution and collaboration. There are conflicts in the file, could you please resolve the conflicts so that we can proceed further with this PR? Thanks |
Closing this PR since it has not been updated for a long time. Please feel free to reopen it if needed. |
I don't believe there are or were any conflicts with this PR? It's a simple deletion and change of an interface implementation. The whole point of this PR was that this line was marking the
Is there a process in place to:
I think it's totally ok if this change breaks (many) builds, because it's flagged as deprecated. This change doesn't really technically do much, if anything, so it seems silly to merge it in. However, tickets like this play to the exact purpose and usage of deprecated docblocks/annotations for the entire framework. Does |
Hi @markshust. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
@markshust I see that deprecation was added 5 years ago in release 2.3.2 d8eb160. I think it's good enough to do a cleanup for deprecated class. |
Just wanted to check to see if there was an update on this to merge it in? |
Description (*)
The
HttpHeadActionInterface
interface has been deprecated and is no longer used. TheHttpGetActionInterface
interface extends this interface; rather, it should extend the baseActionInterface
. TheHttpHeadActionInterface
interface is not used anywhere else in Magento's code.Related Pull Requests
N/A
Fixed Issues (if relevant)
N/A
Manual testing scenarios (*)
Questions or comments
HttpHeadActionInterface
interface?Contribution checklist (*)