Skip to content

Upgrade Doctrine CS, use only for new files#253

Closed
Majkl578 wants to merge 1 commit intodoctrine:masterfrom
Majkl578:cs
Closed

Upgrade Doctrine CS, use only for new files#253
Majkl578 wants to merge 1 commit intodoctrine:masterfrom
Majkl578:cs

Conversation

@Majkl578
Copy link
Copy Markdown
Contributor

6.0.x-dev because I am going to use the new syntax for generics and intersections which only works in 6.0-dev.
Not applying to any code from 1.x era to avoid headaches during rebase conflicts. We will do it later for files that survive refactoring.

@Majkl578 Majkl578 added this to the v2.0.0 milestone Feb 27, 2019
@Majkl578 Majkl578 requested a review from alcaeus February 27, 2019 12:53
Comment thread phpcs.xml.dist
<file>lib/</file>
<file>tests/</file>

<exclude-pattern>lib/Doctrine/Annotations/Annotation/*</exclude-pattern>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lots and lots of excludes: is this worth doing in first place, if half the lib isn't covered at all?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's for the new files we add, e.g. #247, new parser etc.

@alcaeus
Copy link
Copy Markdown
Member

alcaeus commented Mar 5, 2019

Agree with @Ocramius here: if we're only going to exclude most files, let's do this when we don't need to exclude everything. Just to understand the reasoning, why not wait until #247 is merged and then do this for the entire codebase?

@Majkl578
Copy link
Copy Markdown
Contributor Author

Majkl578 commented Mar 5, 2019

@alcaeus Already described the situation in OP:

Not applying to any code from 1.x era to avoid headaches during rebase conflicts. We will do it later for files that survive refactoring.

@alcaeus
Copy link
Copy Markdown
Member

alcaeus commented Mar 5, 2019

After discussion in Slack, we've decided to hold back introducing doctrine/coding-standard until the initial refactoring is done: the current coding standard should not be applied to code that will not survive refactoring, but at the same time we also don't want to apply the coding standard to a small subset of files.

Instead, maintainers and containers are encouraged to make sure their code is formatted in compliance with doctrine/coding-standard. However, code reviews should refrain from enforcing doctrine/coding-standard until it is officially adopted in the repository and checked automatically.

@alcaeus alcaeus closed this Mar 5, 2019
@alcaeus alcaeus removed their request for review March 6, 2019 13:41
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.

3 participants