Skip to content
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

CB-321: No empty comments #221

Merged
merged 3 commits into from Nov 12, 2018

Conversation

bimalkant-lauhny
Copy link
Contributor

Changed SQL and Comment Form to reject empty comments.

@bimalkant-lauhny bimalkant-lauhny force-pushed the no-empty-comments branch 2 times, most recently from f82fe12 to 66bf771 Compare October 31, 2018 16:24
@bimalkant-lauhny
Copy link
Contributor Author

@paramsingh Please review this.

Copy link
Collaborator

@paramsingh paramsingh left a comment

Choose a reason for hiding this comment

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

Looks like you've fixed the stuff we talked about. LGTM.

@@ -0,0 +1,8 @@
BEGIN;

ALTER TABLE "comment_revision"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Generally, we'd have to think about what we want to do with empty comments already in the db before we add a constraint, but I checked and there aren't any empty comments (or any comments whatsoever) in the db so we should be okay.

Copy link
Collaborator

@paramsingh paramsingh left a comment

Choose a reason for hiding this comment

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

Can you change the db version to 13 in db/__init__.py as well?

Copy link
Collaborator

@paramsingh paramsingh left a comment

Choose a reason for hiding this comment

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

Nvm, did it myself.

@paramsingh paramsingh merged commit 0117b44 into metabrainz:master Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants