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
Fix bug when completing "ON parent." clauses #42
Conversation
Previously, the code passed the wrong arguments to `identifiers`. Fixes dbcli#40.
Column(tables=((None, 'tabl', 'a'),), drop_unique=None), | ||
Table(schema='a'), | ||
View(schema='a'), | ||
Function(schema='a', filter=None)) |
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.
I'm not super confident about this test. Did I do it correctly?
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, it is correct
I think we also want to add |
I think that's for completing the word |
It's the first part:
but I can move that out to a separate issue 👍 |
Ah, sorry, I missed that part. |
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.
@getaaron @pgr0ss Thanks for fixing the bug, it looks good to me.
Could you please also update the changelog and add yourself to the AUTHORS.rst?
For the JOIN
and ON
keywords, you can decided to add them in this CR or in a separate CR. Thanks again.
Column(tables=((None, 'tabl', 'a'),), drop_unique=None), | ||
Table(schema='a'), | ||
View(schema='a'), | ||
Function(schema='a', filter=None)) |
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, it is correct
@pgr0ss We appreciate your help and want to give you credit. Please take a moment to put an
|
@zzl0 I don't seem to be able to check those boxes, but I've done both. |
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.
LGTM, Thanks again!
It's fine, thanks |
Previously, the code passed the wrong arguments to
identifiers
.Fixes #40.
This is my first time working in this project, so any and all feedback is welcome!