Skip to content

Add removeCommand() and removeCommands() methods to API.#66

Merged
irazasyed merged 4 commits into
irazasyed:masterfrom
jonnywilliamson:master
Nov 27, 2015
Merged

Add removeCommand() and removeCommands() methods to API.#66
irazasyed merged 4 commits into
irazasyed:masterfrom
jonnywilliamson:master

Conversation

@jonnywilliamson
Copy link
Copy Markdown
Contributor

Added new tests to verify the new methods.

Have also added other new tests in the new suggested format for naming. I have left the other test names alone just so we can see what both look like.

Comment thread tests/ApiTest.php
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Method name ApiTest::it_should_only_allow_a_string_as_the_api_token is not in camel caps format

@irazasyed
Copy link
Copy Markdown
Owner

So nitpick seems to be doing its job and i was bombarded with email notifications :|

@jonnywilliamson
Copy link
Copy Markdown
Contributor Author

Well I'll update my snippet for tests to put a "public" visibility for each function.

As for camel case caps? Mmmm.

Comment thread tests/ApiTest.php
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Method name ApiTest::it_should_throw_exception_if_supplied_command_class_does_not_exist is not in camel caps format

@irazasyed
Copy link
Copy Markdown
Owner

Have to ignore camel case caps check, Let me check their docs.

@jonnywilliamson
Copy link
Copy Markdown
Contributor Author

@irazasyed How about adding the nitpick.json config file and bypassing the test directory?

Example here:
https://gist.github.com/adamwathan/bf382f05c575db4efb6c

@irazasyed
Copy link
Copy Markdown
Owner

@jonnywilliamson I just added but ignoring would completely ignore all the checks. Including like the visibility one (like it suggested) and other PSR2 violations.

irazasyed added a commit that referenced this pull request Nov 27, 2015
Add removeCommand() and removeCommands() methods to API.
@irazasyed irazasyed merged commit 641f391 into irazasyed:master Nov 27, 2015
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