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

Define meaning of errors, warnings and severity levels #20

Open
schmengler opened this issue May 4, 2018 · 5 comments
Open

Define meaning of errors, warnings and severity levels #20

schmengler opened this issue May 4, 2018 · 5 comments
Assignees

Comments

@schmengler
Copy link
Collaborator

phpcs distinguishs between errors and warnings and additionally provides severity levels from 1-9 which are independent from the error/warning levels.

We should pick the severity levels for our own sniffs carefully and implement a guideline to help us doing so.

Some initial suggestions:

  • Errors should be fixed almost without exception. When dealing with legacy core code, it might not be possible, but then an explanatory comment should be required
  • Warnings help finding possible issues and give a rough overview on the extension quality
  • To encourage developers using the Extdn coding standards also for projects, lower severity levels should be used for warnings and errors that are only relevant for extensions, not for projects.

References:

@andrewhowdencom
Copy link

A reasonable parable for this (complete with numeric representations) would be syslog (https://en.wikipedia.org/wiki/Syslog):

0 -> emergency (basically syntax error)
1 -> alert (unused)
2 -> critical (SQL injection detected, use of eval etc)
3 -> error (some sort of bad DI configuration. Maybe use of concrete classes ... somewhere an interface should be used?)
4 -> warning (Maybe ... use of count() instead of getSize()?)
5 -> notice (unsued)
6 -> informational (unusued)
7 -> debug (bork)

@schmengler schmengler added organizational on agenda of hangout This issue will be discussed in the next open hangout and removed debate needed labels May 14, 2018
@fooman
Copy link

fooman commented May 29, 2018

  • Non-negotiable
  • Test
  • Project
  • Default
  • Strict
  • Experimental

@schmengler
Copy link
Collaborator Author

To implement sensible rules for severity levels, we need to figure out use cases first, that would require a certain minimum severity threshold.

We found the following interesting thresholds (no numbers yet):

  • Experimental: Rules that we want to try out first and should be ignored by anyone else
  • Strict: All the best practices for extensions
  • Default: Good extensions should have that
  • Project code: Where some of the rules for extensions do not apply, e.g. extensibility does not matter
  • Test code: Some more rules do not matter

Suggestion to start with

  1. Experimental
  2. ?
  3. Strict
  4. ?
  5. Default
  6. ?
  7. Project
  8. ?
  9. Test
  10. Only non-negotiable rules

That means, the default threshold of 5 checks all the rules with severity 5 or greater. This will exclude strict rules and experimental rules.

I've left gaps for possible further distinctions, like levels of strictness for extensions, or additional use cases

How to determine severity of a rule:

Start from highest to lowest, asking:

  • is it non-negotiable? yes => 10
  • does it apply to test code? yes => 9
  • does it apply to project code? yes => 7
  • does it always apply to extensions? yes => 5
  • does it apply to extensions with stricter requirements? yes => 3
  • is it an experimental rule? yes => 1

@schmengler schmengler removed the on agenda of hangout This issue will be discussed in the next open hangout label Jun 4, 2018
@schmengler schmengler self-assigned this Jun 4, 2018
@fooman
Copy link

fooman commented Jun 5, 2018

Looks great to me. The one I can't judge yet would be the Project level but I am guessing over time this will become clearer.

One additional idea that I like is to tighten the rules over time. So far example a rule would initially start its life as experimental and then gets upgraded to the next levels as we confirm they are useful.

@schmengler
Copy link
Collaborator Author

So far example a rule would initially start its life as experimental and then gets upgraded to the next levels as we confirm they are useful

For experimental rules it's obvious: either they get "upgraded" or dropped at some point. But raising the level step by step sounds interesting too, I like that idea!

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

No branches or pull requests

3 participants