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.

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

[Documentation] PSR12 - File Header #231

Conversation

dingo-d
Copy link
Contributor

@dingo-d dingo-d commented Jan 5, 2024

The PR contains the documentation for the PSR12/Files/FileHeader sniff.

Description

This PR will add the documentation for the above-mentioned sniff, according to the official standard definitions.

Suggested changelog entry

Add documentation for the PSR12 FileHeader sniff

Related issues/external references

Part of #148

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • I have checked there is no other PR open for the same change.
  • I have read the Contribution Guidelines.
  • I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
  • I have added tests to cover my changes.
  • I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added XML documentation for the sniff.

@dingo-d dingo-d marked this pull request as ready for review January 22, 2024 10:07
@dingo-d dingo-d force-pushed the documentation/PSR12-files-file-header branch from 5e588b1 to 526f4b4 Compare January 22, 2024 10:07
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@dingo-d Thank you for working on this! My apologies for taking a while before I got to reviewing the PR.

After reviewing this PR, these are my findings:

  1. While the code samples illustrate the rules and sort of list them in the "Valid" titles, the actual rules themselves are not explained in a <standard> element/elements.
    "Ensures that the PHP file header is properly formatted." is non-descript and doesn't make it clear what "properly formatted" means.
  2. The docs talk about "header blocks" but don't actually explain what code is considered part of the file header. I.e. what is a "header block" ? This makes the docs unclear.
    I think the list of header blocks as per PSR12 will need to be made explicit in the docs.
  3. The docs only contain examples for three out of the four rules the sniff checks for.
    The "Similar statements must be grouped together inside header blocks" rule is missing.
  4. Please review the used code sample for correct use of <em> tags in all relevant places.

@dingo-d dingo-d marked this pull request as draft March 11, 2024 09:23
@jrfnl
Copy link
Member

jrfnl commented Mar 11, 2024

Closing - see: #148 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants