-
Notifications
You must be signed in to change notification settings - Fork 548
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
OHRM5X-1796: Create buzz - entities #1518
Conversation
7a4d8f1
to
8424efd
Compare
/** | ||
* @var int | ||
* | ||
* @ORM\Column(name="number_of_likes", type="integer", nullable=true) |
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.
Just asking. If there is a default value for this field as 0, is there any point of making it nullable?
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.
nice catch. about to define it in next PR. for the moment thought to keep it
/** | ||
* @var DateTime | ||
* | ||
* @ORM\Column(name="comment_time", type="datetime") |
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.
Seems text is required. If so, do we need to make this field nullable?
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.
no need to keep it nullable. I have revert it to make backward compatible with 4.x
* @group Buzz | ||
* @group Entity | ||
*/ | ||
class BuzzPostTest extends EntityTestCase |
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.
Will add test for other entities after merge
Co-authored-by: Manul <manul@orangehrm.us.com>
No description provided.