Skip to content

Document code-style for enums and class constants #16537

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

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Feb 23, 2022

This PR adds some naming conventions.

First of all, class constants. We seem to have an unwritten rule on how class constants should be named. I've made that rule explicit.

Secondly, it adds enumerations to the mix. Those are a new language element introduced in PHP 8.1. Our codebase does not have any enums yet (apart from test fixtures), so I think we should discuss the options before the first PR adding an enum to Symfony's public API arrives.

I took inspiration from the example code from the RFC. The most complete example is probably this one:

enum UserStatus: string
{
  case Pending = 'P';
  case Active = 'A';
  case Suspended = 'S';
  case CanceledByUser = 'C';
}

I propose to adopt the naming conventions used by the RFC because they seem reasonable to me.

Fixes #16538, symfony/symfony#45516.

@nicolas-grekas
Copy link
Member

Works for me. I would add that attributes should be in an Attribute namespace.

@derrabus derrabus force-pushed the improvement/enum-code-style branch 2 times, most recently from e80d6d7 to 3d0cde2 Compare February 23, 2022 11:01
@wouterj
Copy link
Member

wouterj commented Feb 23, 2022

I would add that attributes should be in an Attribute namespace.

In that case, let's move Route from Annotations to Attribute :)

@derrabus
Copy link
Member Author

I would add that attributes should be in an Attribute namespace.

In that case, let's move Route from Annotations to Attribute :)

Yeah, and then there are validation constraints that also violate the rule. We need to document a couple of exceptions to that rule which is why I'd like to separate attribute namespacing from this discussion, if that's okay for you.

@derrabus derrabus force-pushed the improvement/enum-code-style branch from 3d0cde2 to 4cc6393 Compare February 23, 2022 12:56
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 23, 2022

Yeah, and then there are validation constraints that also violate the rule. We need to document a couple of exceptions to that rule which is why I'd like to separate attribute namespacing from this discussion, if that's okay for you.

Works for me also :)
A fallback convention would then be that attributes should be in a dedicated namespace. That would fit Constraints and Annotation(s) namespaces.

@stof
Copy link
Member

stof commented Feb 24, 2022

The case of Constraints is special, because they are not only attributes. They are the API of the component directly.

@javiereguiluz
Copy link
Member

Thank you Alexander.

@javiereguiluz javiereguiluz merged commit 3ce2c1a into symfony:4.4 Feb 25, 2022
@derrabus derrabus deleted the improvement/enum-code-style branch February 25, 2022 19:24
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.