-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Add a module manager to the Magento Framework API #18748
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
Add a module manager to the Magento Framework API #18748
Conversation
Hi @navarr. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
#squashtoberfest |
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.
We are encouraged to use strict typing. Thus, I suggest adding parameter type and return value for the isEnabled()
method.
Added strict_types declaration to Manager.php, and added parameter and return typehints to isEnabled method
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.
good job!
Hi @aleron75, thank you for the review. |
@sivaschenko Alright. I typically attend so that shouldn't be an issue. |
API Annotation should be added to the interface in Magento 2.4
Pull request was approved to be merged to 2.3-develop at Public Architectural Discussion meeting magento/architecture#165 (comment) |
Hi @sivaschenko, thank you for the review. |
88675ba
to
22bed61
Compare
22bed61
to
1f9ab52
Compare
1f9ab52
to
4a0e862
Compare
4a0e862
to
20f0c16
Compare
20f0c16
to
6a3c90e
Compare
6a3c90e
to
5e14e8a
Compare
Hi @navarr, thank you for your contribution! |
This is a useful addition for feature detection and enablement for third party modules (such as "activate this functionality if such and such GiftCard extension exists"
This is an update to PR #12677 to be based on 2.3