8000 226 fix populate connections bug by grbritz · Pull Request #227 · tableau/server-client-python · GitHub
[go: up one dir, main page]
Skip to content

226 fix populate connections bug #227

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 21, 2017
Merged

226 fix populate connections bug #227

merged 6 commits into from
Sep 21, 2017

Conversation

grbritz
Copy link
Contributor
@grbritz grbritz commented Sep 20, 2017

This address issue #226

t8y8
t8y8 approved these changes Sep 21, 2017
View reviewed changes
Copy link
Collaborator
@t8y8 t8y8 left a comment

Choose a reason for hiding this comment

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

🚀

@RussTheAerialist .

Looks like we missed an endpoint :'(

I'll file an item to investigate if any others are out there and how test coverage can catch it.

@t8y8
8000 Copy link
Collaborator
t8y8 commented Sep 21, 2017

Ok, I just confirmed we have no test coverage on populate_connections at all, that's why it didn't get caught.

@graysonarts
Copy link
Contributor
graysonarts commented Sep 21, 2017 via email

@t8y8
Copy link
Collaborator
t8y8 commented Sep 21, 2017

@grbritz might not know how to mock it all out. I will see if I can get him the code tonight and he can copy/paste

@graysonarts
Copy link
Contributor

🚢 okay then I'm going to merge this PR and get a new release out this morning to make sure we don't break too many people. @t8y8 are the tests something you could work on? I have some other bugs on my plate right now that need to take priority for various reasons.

@graysonarts graysonarts merged commit 73f8415 into tableau:development Sep 21, 2017
@grbritz
Copy link
Contributor Author
grbritz commented Sep 21, 2017

@RussTheAerialist Thanks for merging. I'll see if I can figure out how to add the mocks but I have other more urgent tasks I need to get to today. FYI if you didn't know, my PR was into the development branch, so users of the master branch will still be experiencing the issue.

@graysonarts
Copy link
Contributor

Thanks @grbritz ! Let me or @t8y8 know if you need any help (feel free to drop by), I'm working on getting it into master #228 and then will release a new one on pypi.

@t8y8
Copy link
Collaborator
t8y8 commented Sep 21, 2017

@RussTheAerialist yeah I'll work on it -- I need those tests for 'pager all the things' anyway!

bryceglarsen pushed a commit to bryceglarsen/server-client-python that referenced this pull request Dec 28, 2023
@bcantoni
Merge pull request tableau#227 from tableau/minor-cleanup
f7b8aa2 
Minor doc improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t8y8 t8y8 t8y8 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@grbritz @t8y8 @graysonarts
0