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

PR: Include the output toolkit api #39

Merged
merged 4 commits into from
Feb 20, 2017
Merged

Conversation

goanpeca
Copy link
Contributor

@goanpeca goanpeca commented Feb 20, 2017

Fixes #40
Fixes #32

@goanpeca goanpeca self-assigned this Feb 20, 2017
@bemcdonnell
Copy link
Member

@goanpeca Is this fully backwards compatible with py2.7?

@goanpeca
Copy link
Contributor Author

goanpeca commented Feb 20, 2017

Yes, we just need an extra dependency (enum34). I am adding it to the setup.py, no worries ;-)

@bemcdonnell
Copy link
Member

#40 Raised Issue Here

@bemcdonnell bemcdonnell self-requested a review February 20, 2017 21:59
@bemcdonnell
Copy link
Member

@goanpeca what is the common practice here for including source code (the actual C code)?

@bemcdonnell
Copy link
Member

bemcdonnell commented Feb 20, 2017

I only ask since the python wrapper is leveraging some community based tools

@bemcdonnell bemcdonnell added this to the v0.3 milestone Feb 20, 2017
@goanpeca
Copy link
Contributor Author

goanpeca commented Feb 20, 2017

what is the common practice here for including source code (the actual C code)?

Well we normally bundle it in a src folder, dont know if that is what you meant

Copy link
Member

@bemcdonnell bemcdonnell left a comment

Choose a reason for hiding this comment

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

Thanks

@goanpeca
Copy link
Contributor Author

goanpeca commented Feb 20, 2017

@bemcdonnell should we merge?

@bemcdonnell
Copy link
Member

Sure!

@bemcdonnell bemcdonnell merged commit ba7a15c into master Feb 20, 2017
@bemcdonnell bemcdonnell deleted the feature/swmmoutput branch February 20, 2017 22:34
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.

None yet

2 participants