Skip to content

Conversation

kalpmehta
Copy link
Contributor

@kalpmehta kalpmehta commented Jun 26, 2019

Description (*)

Security.txt module provides an implementation of the security.txt standard (https://tools.ietf.org/html/draft-foudil-securitytxt-06) in Magento.

README: https://github.com/kalpmehta/securitytxt/blob/master/Kalpesh/Securitytxt/README.md

Fixed Issues (if relevant)

  1. magento/magento2#<issue_number>: Issue title

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

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)
  • All automated tests passed successfully (all builds are green)

@ishakhsuvarov
Copy link
Contributor

Some general comments:

  • Looks like included images are unused
  • All classes must be declared strict types and all methods must have strict parameter and return types in their signatures.
  • Would be nice to run code through phpcs with the Magento rulest: https://github.com/magento/magento-coding-standard
  • It's best to avoid protected visibility
  • To accept the extension as a core extension we would need to change namespace to Magento and update copyrights with those used in core.

@kalpmehta
Copy link
Contributor Author

Thanks @ishakhsuvarov for the detailed review! I will work on the recommendations.

@gwillem
Copy link

gwillem commented Jul 24, 2019

Thanks @kalpmehta for this contribution and @ishakhsuvarov for quality reviewing!

Copy link
Contributor

@ishakhsuvarov ishakhsuvarov left a comment

Choose a reason for hiding this comment

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

Looks like Securitytxt/.DS_Store is mistakenly added to version control

@ishakhsuvarov ishakhsuvarov merged commit 0793e6d into magento:2.3-develop Nov 1, 2019
mmansoor-magento pushed a commit that referenced this pull request Oct 16, 2020
#276 2fa redirect checks wrong permissions
magento-devops-reposync-svc pushed a commit that referenced this pull request May 24, 2022
CABPI-390: Do not allow to enable AdminAdobeIms when 2FA is disabled on IMS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants