-
Notifications
You must be signed in to change notification settings - Fork 133
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
Bugfixes #1012
Bugfixes #1012
Conversation
…instance checks use `isinstance()`
@bmos sorry for taking so long to respond, I've been traveling. Have you made any changes to the docs other than linting? It's hard to see in the diffs if there's anything other than line length changes. Otherwise everything looks good to me. |
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, just waiting to confirm that there aren't any docs content changes before merging.
Only formatting changes and can revert those files if preferred. |
No need to revert, just wanted to make sure I wasn't missing anything. |
oauth2client==4.1.3 | ||
google-auth==2.6.2 | ||
dbt_redshift==1.4.0 | ||
docutils<0.18,>=0.14 |
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.
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 think it's been that way since before my time and the only reason that it's changed by this PR is because I alphabetized the requirements.
I would try removing it from requirements altogether and let Sphinx install it as a dependency.
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.
git blame letting me down :( I will give it a try
In more in-depth refactoring I'm looking into, I have come across a number of changes needed to get tests passing, resolve test warnings, and obey lints. As requested in #1009, I have taken all the minor bugfixes and compiled them here.
ddccd1e also fixes the only remaining issue holding back 3.11 support! So I added it to setup.py and the test suite to demonstrate that.