-
Notifications
You must be signed in to change notification settings - Fork 144
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
[TASK] Add trait providing standard implementation of Commentable
#1206
Conversation
c3f5699
to
aacf98b
Compare
aacf98b
to
fa0e11c
Compare
* | ||
* @dataProvider provideCommentArray | ||
*/ | ||
public function getCommentsAfterEmptyArrayOfCommentsAddedReturnsOriginalComments(array $comments): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can describe the effects instead of focusing on the methods:
public function getCommentsAfterEmptyArrayOfCommentsAddedReturnsOriginalComments(array $comments): void | |
public function setCommentsWithEmptyArrayKeepsOriginalCommentsUnchanged(array $comments): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from getCommentsInitiallyReturnsEmptyArray()
, all the test methods focus on either addComments()
(first group) or setComments()
(last group). So perhaps they all be renamed like addCommentsDoesXYZ()
and setCommentsDoesXYZ
- WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'd like that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
a0631c6
to
c202639
Compare
There's a leftover |
Co-authored-by: Oliver Klee <[email protected]>
c202639
to
94a17b4
Compare
I've replied to the open comment with a question. I've fixed the leftover |
In most cases this is either `addComments` or `setComments`, rather than `getComments`.
Resolves #813.