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

Basic connection retargeting #65

Merged
merged 2 commits into from
Jul 27, 2016
Merged

Basic connection retargeting #65

merged 2 commits into from
Jul 27, 2016

Conversation

t8y8
Copy link
Contributor

@t8y8 t8y8 commented Jul 26, 2016

First attempt for feedback, really simple implementation.

A tiny part of the full #34

  • Following the same setters as elsewhere
  • I copied the good dbclasses from a cpp file, and removed the ones I didn't think are out of beta yet
  • Tests

@@ -4,6 +4,61 @@
#
###############################################################################

KNOWN_DB_CLASSES = ('msaccess',
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh ew. I mean, I understand why, but maybe move it into it's own file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hahahaha. But getting them all lined up took the most time for this PR! (And writing the regex to extract them)

constants.py? Or something more specific?

Copy link
Contributor

Choose a reason for hiding this comment

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

well, if we call it something like dbclass and bundle the IsValidDbClass function in the same file, I think that's be good, because then you'd never really expose the constant, you'd always just use the IsValid* function

@graysonarts
Copy link
Contributor

🚀

@t8y8 t8y8 merged commit 44ca620 into tableau:development Jul 27, 2016
@t8y8 t8y8 deleted the feature-connection-retarget branch July 27, 2016 03:20
@graysonarts graysonarts modified the milestone: 2016.08 release Aug 29, 2016
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.

2 participants