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

feat(comment): Add comment spec for columns #15

Merged
merged 3 commits into from
Dec 11, 2019
Merged

feat(comment): Add comment spec for columns #15

merged 3 commits into from
Dec 11, 2019

Conversation

Nosfistis
Copy link
Contributor

@Nosfistis Nosfistis commented Jul 31, 2018

Columns can have comment if the 'comment' property is set in the column definition.

Relates to db-migrate/node-db-migrate#558

Columns can have comment if the 'comment' property is set in the column definition.

Relates to #558
@Nosfistis Nosfistis closed this Jul 31, 2018
Columns can have comment if the 'comment' property is set in the column definition.

Relates to #558

Signed-off-by: Mike Drakoulelis <mikedrakoulelis@yahoo.gr>
@Nosfistis Nosfistis reopened this Jul 31, 2018
@Nosfistis
Copy link
Contributor Author

I have made the required changes for the column comment, however I cannot make the tests to pass (even in master branch) so I cannot add further tests for comments. It seems that no tables are created, since event table is never found on initial check.

Also, it might be difficult to check for the existence of comments, since db-meta does not support getting column comment, a separate SQL has to be made in test.

I would like some help so that I can implement the required tests, if needed.

@wzrdtales
Copy link
Member

I will review this in the next days, should I for some reason do not please ping me @miced

@Nosfistis
Copy link
Contributor Author

@wzrdtales any update?

@Nosfistis
Copy link
Contributor Author

Any update on this PR? @wzrdtales

@wzrdtales wzrdtales merged commit beca789 into db-migrate:master Dec 11, 2019
@Nosfistis Nosfistis deleted the comments branch December 11, 2019 10:23
@williamdes
Copy link

Hey
You did forget that french people add ' in comments
But thank you for the implementation !

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