Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Conversation

lenaorobei
Copy link
Contributor

@lenaorobei lenaorobei commented Apr 26, 2019

- added info about Magento Coding Standard
- fixed outdated information
@magento-engcom-team
Copy link

@lenaorobei thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Apr 26, 2019

CLA assistant check
All committers have signed the CLA.

@devops-devdocs
Copy link
Collaborator

An admin must run tests on this PR before it can be merged.

@lenaorobei lenaorobei changed the title REPO-203: [EQP][Sniffs Consolidation] Update Magento DevDocs pages Updated corresponding pages due to with Magento Coding Standard release Apr 26, 2019
@dshevtsov dshevtsov added 2.1.x 2.2.x 2.3.x Magento 2.3 related changes Major Update Significant original updates to existing content labels Apr 26, 2019
@lenaorobei
Copy link
Contributor Author

lenaorobei commented Apr 26, 2019

Adding more clarity here:

  • Moved code-standard-sniffers.html content under code-standard-php.html page, because Code Sniffer is a tool that allows to check the code compliance with specific Coding Standard.
  • Replaced links to outdated MEQP Coding Standard with new one consolidated Magento Coding Standard.
  • Fixed Templates XSS security with relevant for 2.3 approach of testing phtml templates.

@jeff-matthews jeff-matthews self-assigned this Apr 26, 2019
Otherwise, you must apply these standards and requirements through rigorous code review.
Magento Coding Standard provides the set of rules which cover following:
* [PSR-1]{:target="_blank"} and [PSR-2]{:target="_blank"} compliance
* the use of insecure functions
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to list all custom rules here? It may be difficult to maintain these docs up-to-date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see an issue here. It looks good to me that we explicitly declare PHP standard we are following.

@jcalcaben jcalcaben added the Waiting for Response Waiting for response from internal/external parties label Apr 30, 2019
Copy link
Contributor

@jeff-matthews jeff-matthews left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @lenaorobei! Please see my minor editorial suggestions.

@jeff-matthews jeff-matthews removed the Waiting for Response Waiting for response from internal/external parties label May 1, 2019
jeff-matthews and others added 2 commits May 2, 2019 09:28
Co-Authored-By: lenaorobei <oorobei@magento.com>
Co-Authored-By: lenaorobei <oorobei@magento.com>
@lenaorobei
Copy link
Contributor Author

@jeff-matthews thank you fo the review and suggestions.

@jeff-matthews
Copy link
Contributor

running tests

@jeff-matthews
Copy link
Contributor

Tests failed. Here's the log.

  • rake test:cicd
    �[35mChecking links with htmlproofer...�[0m
    Running ["LinkChecker::DoubleSlashCheck", "ImageCheck", "ScriptCheck", "LinkCheck"] on ["_site"] on *.html...

Ran on 4098 files!

- internally linking to /guides/v2.1/coding-standards/code-standard-sniffers.html, which does not exist
  *  _site/guides/v2.1/coding-standards/bk-coding-standards.html (line 208)
     <a href="/guides/v2.1/coding-standards/code-standard-sniffers.html">PHP code sniffers</a>
- internally linking to /guides/v2.2/coding-standards/code-standard-sniffers.html, which does not exist
  *  _site/guides/v2.2/coding-standards/bk-coding-standards.html (line 208)
     <a href="/guides/v2.2/coding-standards/code-standard-sniffers.html">PHP code sniffers</a>
- internally linking to /guides/v2.3/coding-standards/code-standard-sniffers.html, which does not exist
  *  _site/guides/v2.3/coding-standards/bk-coding-standards.html (line 208)
     <a href="/guides/v2.3/coding-standards/code-standard-sniffers.html">PHP code sniffers</a>
rake aborted!
HTML-Proofer found 3 failures!```

@jeff-matthews
Copy link
Contributor

jeff-matthews commented May 2, 2019

@lenaorobei, please remove the links to the deleted Code Sniffer file in the following files:

  • guides/v2.1/coding-standards/bk-coding-standards.md
  • guides/v2.2/test/testing.md
  • guides/v2.3/test/testing.md
  • _data/whats-new.yml

@lenaorobei
Copy link
Contributor Author

@jeff-matthews, done.

@jeff-matthews
Copy link
Contributor

running tests

@jeff-matthews jeff-matthews merged commit 8fe337f into magento:master May 2, 2019
@ghost
Copy link

ghost commented May 2, 2019

Hi @lenaorobei, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2.1.x 2.2.x 2.3.x Magento 2.3 related changes Major Update Significant original updates to existing content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants