Skip to content

Enhancement: Use doctrine/coding-standard#177

Closed
localheinz wants to merge 1 commit intodoctrine:masterfrom
localheinz:feature/coding-standard
Closed

Enhancement: Use doctrine/coding-standard#177
localheinz wants to merge 1 commit intodoctrine:masterfrom
localheinz:feature/coding-standard

Conversation

@localheinz
Copy link
Copy Markdown
Contributor

@localheinz localheinz commented Feb 3, 2018

❗️ Blocked by #174.

This PR

  • requires doctrine/coding-standard and adds a coding standard stage
  • runs phpcbf (so far, 3x)

Comment thread composer.json Outdated
},
"require-dev": {
"doctrine/cache": "1.*",
"doctrine/coding-standard": "^2.0.0",
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.

We can use ^2.1.0 here

@localheinz localheinz force-pushed the feature/coding-standard branch from 564555d to e86da8b Compare February 3, 2018 17:19
@localheinz localheinz mentioned this pull request Feb 3, 2018
1 task
@localheinz localheinz force-pushed the feature/coding-standard branch 2 times, most recently from fbd3cc9 to 620784b Compare February 3, 2018 17:23
@localheinz
Copy link
Copy Markdown
Contributor Author

@lcobucci

Build is failing because of phpcs sniffing more than phpcbf can fix, what would you recommend how to make the build pass?

@lcobucci
Copy link
Copy Markdown
Member

lcobucci commented Feb 3, 2018

@localheinz I think you forgot to add phpcs.xml.dist to your branch, also it seems some of the fixes were not applied properly (phpunit is failing).

@Majkl578
Copy link
Copy Markdown
Contributor

Majkl578 commented Feb 3, 2018

Please don't do this now, it would terribly break Annotations 2.0 PR: #75.

@localheinz localheinz force-pushed the feature/coding-standard branch from 620784b to 7616348 Compare February 3, 2018 17:38
@lcobucci
Copy link
Copy Markdown
Member

lcobucci commented Feb 3, 2018

@localheinz there will be things that cannot be automatically fixed as well, for these we would need manual changes.

@Majkl578 it is already conflicting with master...

merge result
Auto-merging tests/Fixtures/TraitWithAnnotatedMethod.php
Auto-merging tests/Fixtures/NamespacedSingleClassLOC1000.php
Auto-merging tests/Fixtures/InvalidAnnotationUsageClass.php
CONFLICT (rename/delete): tests/Doctrine/Tests/Common/Annotations/Fixtures/IntefaceWithConstants.php deleted in HEAD and renamed to tests/Fixtures/IntefaceWithConstants.php in 2.0. Version 2.0 of tests/Fixtures/IntefaceWithConstants.php left in tree.
Auto-merging tests/Fixtures/DummyClass.php
Auto-merging tests/Fixtures/Controller.php
Auto-merging tests/Fixtures/ClassWithValidAnnotationTarget.php
Auto-merging tests/Fixtures/ClassWithRequire.php
Auto-merging tests/Fixtures/ClassWithInvalidAnnotationTargetAtProperty.php
Auto-merging tests/Fixtures/ClassWithInvalidAnnotationTargetAtMethod.php
Auto-merging tests/Fixtures/ClassWithInvalidAnnotationTargetAtClass.php
Auto-merging tests/Fixtures/ClassWithIgnoreAnnotation.php
Auto-merging tests/Fixtures/ClassWithConstants.php
Auto-merging tests/Fixtures/ClassWithClosure.php
Auto-merging tests/Fixtures/ClassWithAtInDescriptionAndAnnotation.php
Auto-merging tests/Fixtures/ClassWithAnnotationWithVarType.php
Auto-merging tests/Fixtures/ClassWithAnnotationWithTargetSyntaxError.php
Auto-merging tests/Fixtures/ClassWithAnnotationEnum.php
Auto-merging tests/Fixtures/ClassUsesTrait.php
Auto-merging tests/Fixtures/ClassOverwritesTrait.php
Auto-merging tests/Fixtures/ClassNoNamespaceNoComment.php
Auto-merging tests/Fixtures/ClassDDC1660.php
Auto-merging tests/Fixtures/Api.php
Auto-merging tests/Fixtures/Annotation/Template.php
CONFLICT (content): Merge conflict in tests/Fixtures/Annotation/Template.php
Auto-merging tests/Fixtures/Annotation/Secure.php
Auto-merging tests/Fixtures/Annotation/Route.php
Auto-merging tests/Fixtures/Annotation/AnnotationWithVarType.php
CONFLICT (content): Merge conflict in tests/Fixtures/Annotation/AnnotationWithVarType.php
Auto-merging tests/Fixtures/Annotation/AnnotationWithTargetSyntaxError.php
Auto-merging tests/Fixtures/Annotation/AnnotationWithConstants.php
Auto-merging tests/Fixtures/Annotation/AnnotationTargetPropertyMethod.php
Auto-merging tests/Fixtures/Annotation/AnnotationTargetMethod.php
Auto-merging tests/Fixtures/Annotation/AnnotationTargetClass.php
Auto-merging tests/Fixtures/Annotation/AnnotationTargetAnnotation.php
Auto-merging tests/Fixtures/Annotation/AnnotationTargetAll.php
Auto-merging tests/Fixtures/Annotation/AnnotationEnumLiteralInvalid.php
Auto-merging tests/Fixtures/Annotation/AnnotationEnumLiteral.php
Auto-merging tests/Fixtures/Annotation/AnnotationEnumInvalid.php
Auto-merging tests/Fixtures/Annotation/AnnotationEnum.php
Auto-merging tests/Fixtures/Annotation/AnnotWithDefaultValue.php
CONFLICT (modify/delete): tests/Doctrine/Tests/TestInit.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/TestInit.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/DoctrineTestCase.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/DoctrineTestCase.php left in tree.
Removing tests/Doctrine/Tests/Common/Annotations/TopLevelAnnotation.php
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/Ticket/DCOM58Test.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/Ticket/DCOM58Test.php left in tree.
Removing tests/Doctrine/Tests/Common/Annotations/Ticket/DCOM58Entity.php
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/Ticket/DCOM55Test.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/Ticket/DCOM55Test.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/SimpleAnnotationReaderTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/SimpleAnnotationReaderTest.php left in tree.
Removing tests/Doctrine/Tests/Common/Annotations/ReservedKeywordsClasses.php
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/PhpParserTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/PhpParserTest.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/PerformanceTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/PerformanceTest.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/Fixtures/TestInterface.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/Fixtures/TestInterface.php left in tree.
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/NonNamespacedClass.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/NoAnnotation.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/NamespaceWithClosureDeclaration.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/NamespaceAndClassCommentedOut.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/MultipleImportsInUseStatement.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/MultipleClassesInFile.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/InvalidAnnotationUsageButIgnoredClass.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/IgnoreAnnotationClass.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/GlobalNamespacesPerFileWithClassAsLast.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/GlobalNamespacesPerFileWithClassAsFirst.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/EqualNamespacesPerFileWithClassAsLast.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/EqualNamespacesPerFileWithClassAsFirst.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/DifferentNamespacesPerFileWithClassAsLast.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/DifferentNamespacesPerFileWithClassAsFirst.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/ClassWithFullyQualifiedUseStatements.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/ClassWithClassAnnotationOnly.php
CONFLICT (rename/rename): Rename "tests/Doctrine/Tests/Common/Annotations/Fixtures/AnnotationWithRequiredAttributesWithoutContructor.php"->"tests/Doctrine/Tests/Common/Annotations/Fixtures/AnnotationWithRequiredAttributesWithoutConstructor.php" in branch "HEAD" rename "tests/Doctrine/Tests/Common/Annotations/Fixtures/AnnotationWithRequiredAttributesWithoutContructor.php"->"tests/Fixtures/Annotation/AnnotationWithRequiredAttributesWithoutContructor.php" in "2.0"
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/Fixtures/AnnotationWithRequiredAttributes.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/Fixtures/AnnotationWithRequiredAttributes.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/Fixtures/AnnotationWithAttributes.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/Fixtures/AnnotationWithAttributes.php left in tree.
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/Annotation/Version.php
Removing tests/Doctrine/Tests/Common/Annotations/Fixtures/Annotation/Autoload.php
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/FileCacheReaderTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/FileCacheReaderTest.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/DocParserTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/DocParserTest.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/DocLexerTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/DocLexerTest.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/CachedReaderTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/CachedReaderTest.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/AnnotationReaderTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/AnnotationReaderTest.php left in tree.
CONFLICT (modify/delete): tests/Doctrine/Tests/Common/Annotations/AbstractReaderTest.php deleted in 2.0 and modified in HEAD. Version HEAD of tests/Doctrine/Tests/Common/Annotations/AbstractReaderTest.php left in tree.
Auto-merging tests/Annotation/TargetTest.php
CONFLICT (content): Merge conflict in tests/Annotation/TargetTest.php
Auto-merging src/Reference.php
Auto-merging src/Reader.php
Auto-merging src/Parser/TokenParser.php
CONFLICT (content): Merge conflict in src/Parser/TokenParser.php
Auto-merging src/Parser/PhpParser.php
CONFLICT (content): Merge conflict in src/Parser/PhpParser.php
Auto-merging src/Metadata/ClassMetadata.php
Auto-merging src/Annotation/Target.php
Auto-merging src/Annotation/Required.php
Auto-merging src/Annotation/IgnoreAnnotation.php
Auto-merging src/Annotation/Enum.php
Auto-merging src/Annotation.php
Removing lib/Doctrine/Common/Annotations/SimpleAnnotationReader.php
Removing lib/Doctrine/Common/Annotations/IndexedReader.php
CONFLICT (modify/delete): lib/Doctrine/Common/Annotations/FileCacheReader.php deleted in 2.0 and modified in HEAD. Version HEAD of lib/Doctrine/Common/Annotations/FileCacheReader.php left in tree.
CONFLICT (modify/delete): lib/Doctrine/Common/Annotations/DocParser.php deleted in 2.0 and modified in HEAD. Version HEAD of lib/Doctrine/Common/Annotations/DocParser.php left in tree.
Removing lib/Doctrine/Common/Annotations/DocLexer.php
CONFLICT (modify/delete): lib/Doctrine/Common/Annotations/CachedReader.php deleted in 2.0 and modified in HEAD. Version HEAD of lib/Doctrine/Common/Annotations/CachedReader.php left in tree.
CONFLICT (modify/delete): lib/Doctrine/Common/Annotations/AnnotationRegistry.php deleted in 2.0 and modified in HEAD. Version HEAD of lib/Doctrine/Common/Annotations/AnnotationRegistry.php left in tree.
CONFLICT (modify/delete): lib/Doctrine/Common/Annotations/AnnotationReader.php deleted in 2.0 and modified in HEAD. Version HEAD of lib/Doctrine/Common/Annotations/AnnotationReader.php left in tree.
Removing lib/Doctrine/Common/Annotations/AnnotationException.php
Auto-merging composer.json
CONFLICT (content): Merge conflict in composer.json
Auto-merging README.md
CONFLICT (content): Merge conflict in README.md
Auto-merging .travis.yml
CONFLICT (content): Merge conflict in .travis.yml
Automatic merge failed; fix conflicts and then commit the result.

@localheinz
Copy link
Copy Markdown
Contributor Author

@lcobucci @Majkl578

If you prefer, I can hold this off until later, just let me know when it would be a good time to work on this.

@Majkl578
Copy link
Copy Markdown
Contributor

Majkl578 commented Feb 3, 2018

@lcobucci Last time I checked it was afaik fixable. If we apply CS now, it would be nightmare and I don't want to scrub that work completely. :/ I think we should first decide what the fate of that PR will be, merge/pick parts of it and then apply CS on top, that would be most practical imho.

@localheinz localheinz force-pushed the feature/coding-standard branch from 7616348 to e017d5a Compare February 3, 2018 17:45
@Majkl578
Copy link
Copy Markdown
Contributor

Majkl578 commented Feb 3, 2018

Btw I hope we will salvage it soon, once ORM mapping refactoring is complete. 👍 The work done there is pretty important and should not be lost imho.

@Majkl578 Majkl578 added this to the v2.0.0 milestone Apr 27, 2018
@Majkl578
Copy link
Copy Markdown
Contributor

Majkl578 commented Oct 8, 2018

@localheinz If you want to move this forward, here's my suggestion:

  • Use doctrine/coding-standard 5.0
  • Disable BC breaking sniffs (for now, we will handle those later separately):
    <rule ref="Doctrine">
        <exclude name="SlevomatCodingStandard.TypeHints.DeclareStrictTypes"/>
        <exclude name="SlevomatCodingStandard.TypeHints.TypeHintDeclaration.MissingParameterTypeHint"/>
        <exclude name="SlevomatCodingStandard.TypeHints.TypeHintDeclaration.MissingReturnTypeHint"/>
        <exclude name="SlevomatCodingStandard.Classes.SuperfluousAbstractClassNaming"/>
        <exclude name="SlevomatCodingStandard.Classes.SuperfluousExceptionNaming"/>
    </rule>
  • Exclude the code that will not end up in 2.0 anyway (so we don't clutter the history/diffs with those):
    <exclude-pattern>lib/Doctrine/Annotations/DocLexer.php</exclude-pattern>
    <exclude-pattern>lib/Doctrine/Annotations/DocParser.php</exclude-pattern>
    <exclude-pattern>tests/Doctrine/Tests/Annotations/DocLexerTest.php</exclude-pattern>
    <exclude-pattern>tests/Doctrine/Tests/Annotations/DocParserTest.php</exclude-pattern>

Then we won't merge it right away, but I'll rebase our working branch for 2.0 stuff on top of these changes. Sounds good to you? 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants