Skip to content

Add special Abstract prefix not usage for PHPUnit test cases #8600

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
Nov 12, 2017

Conversation

soullivaneuh
Copy link
Contributor

@soullivaneuh soullivaneuh commented Nov 7, 2017

As we can see here:
image

You are not using the Abstract prefix at all for the test cases.
I assume this is to follow PHPUnit conventions on this part?

In this case, the coding standard may be more accurate about this.

@@ -190,7 +190,7 @@ Naming Conventions

* Use namespaces for all classes;

* Prefix abstract classes with ``Abstract``. Please note some early Symfony classes
* Prefix all abstract classes with ``Abstract`` excepting PHPUnit ``*TestCase``. Please note some early Symfony classes
Copy link
Contributor

Choose a reason for hiding this comment

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

"except"

@@ -190,7 +190,7 @@ Naming Conventions

* Use namespaces for all classes;

* Prefix abstract classes with ``Abstract``. Please note some early Symfony classes
* Prefix all abstract classes with ``Abstract`` excepting PHPUnit ``*TestCase``. Please note some early Symfony classes
Copy link
Contributor

Choose a reason for hiding this comment

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

@xabbuh xabbuh added this to the 2.7 milestone Nov 8, 2017
@weaverryan
Copy link
Member

Thanks @soullivaneuh!

@weaverryan weaverryan merged commit f4d122a into symfony:3.3 Nov 12, 2017
weaverryan added a commit that referenced this pull request Nov 12, 2017
…ses (Soullivaneuh)

This PR was merged into the 3.3 branch.

Discussion
----------

Add special Abstract prefix not usage for PHPUnit test cases

As we can see here:
![image](https://user-images.githubusercontent.com/1698357/32507068-bddf178a-c3e6-11e7-8d28-05f3b18ea18c.png)

You are not using the `Abstract` prefix at all for the test cases.
I assume this is to follow PHPUnit conventions on this part?

In this case, the coding standard may be more accurate about this.

Commits
-------

f4d122a Add special Abstract prefix not usage for PHPUnit test cases
@weaverryan
Copy link
Member

Just a note: I forgot to switch the branch to 2.7 on merge, but backported it manually.

weaverryan added a commit that referenced this pull request Nov 12, 2017
weaverryan added a commit that referenced this pull request Nov 12, 2017
* 2.7:
  Added Best Practice guideline for partials/include
  Backporting #8600
  Move `schema_filter` option to the correct place
  Fix typo in "advanced ACL concepts"
weaverryan added a commit that referenced this pull request Nov 12, 2017
* 2.8:
  Added Best Practice guideline for partials/include
  Backporting #8600
  Move `schema_filter` option to the correct place
  Fix typo in "advanced ACL concepts"
weaverryan added a commit that referenced this pull request Nov 12, 2017
* 3.3:
  Added Best Practice guideline for partials/include
  Backporting #8600
  [#8605] Updated the article a little more, using the tests/ directory.
  fix paths
  Move `schema_filter` option to the correct place
  Fix typo in "advanced ACL concepts"
  Add special Abstract prefix not usage for PHPUnit test cases
weaverryan added a commit that referenced this pull request Nov 12, 2017
* 3.4:
  Added Best Practice guideline for partials/include
  Backporting #8600
  [#8605] Updated the article a little more, using the tests/ directory.
  fix paths
  Move `schema_filter` option to the correct place
  Fix typo in "advanced ACL concepts"
  Add special Abstract prefix not usage for PHPUnit test cases
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.

6 participants