MyIntervals / emogrifier

Converts CSS styles into inline style attributes in your HTML code.
https://www.myintervals.com/emogrifier.php
MIT License
905 stars 153 forks source link

Review braces_position coding style enforcement in PHP CS Fixer #1261

Open JakeQZ opened 3 months ago

JakeQZ commented 3 months ago

A single-line function/method declaration has the code-block opening brace on the next line:

function a()
{

Multiline declarations have this on the same line as the closing parenthesis for the arguments:

function longFunctionName(
  string $param
) {

This works for readability, because the ) { don't interfere with the first line of the function.

But consider a function with a type-hinted return value:

function longFunctionName(
  string $param
): TypeName {
  $value = \intval($param)';

  if ($value < 0) {

For readability, it seems like it would be better if the code block opening brance ({) were on a new line.

However, the current PHP CS Fixer rules we have in place forbid that. Ref: https://github.com/MyIntervals/emogrifier/pull/1236#discussion_r1612509116

oliverklee commented 3 months ago

In general, I'd suggest to update our CS-Fixer rules to the PER-2 set (which seems to be the current recommended common ground for PHP projects) including the risky rules and drop our custom rules. This way, we'll be consistent with other projects while reducing the maintenance effort for our own configuration.

What do you think?

JakeQZ commented 3 months ago

What do you think?

I think that makes sense (though I don't know what the 'risky rules' are offhand).

Edit: oh, I see the 'risky rules' relate to migration between PHP versions.