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

Refactor wrapper scripts #318

Merged
merged 2 commits into from
Aug 10, 2016
Merged

Refactor wrapper scripts #318

merged 2 commits into from
Aug 10, 2016

Conversation

dirk-thomas
Copy link
Member

Fixes #301.

@dirk-thomas dirk-thomas self-assigned this Aug 9, 2016
@@ -0,0 +1,142 @@
#!/usr/bin/env python3

Copy link
Member

Choose a reason for hiding this comment

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

This and below should get a license header.

Copy link
Member Author

Choose a reason for hiding this comment

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

None of the other source files in this repo has any so I wouldn't add it for these two files which have only been moved in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Well we should add it to all the files. I just noticed for these files because I saw the full diff. We can merge this as is I've opened a new ticket #320

@tfoote
Copy link
Member

tfoote commented Aug 9, 2016

That's a nice solution. One comment about the license headers, otherwise +1

It looks like we should actually go through and fill in licenses in general.

@dirk-thomas dirk-thomas merged commit 0f49af1 into master Aug 10, 2016
@dirk-thomas dirk-thomas deleted the refactor_wrapper_scripts branch August 10, 2016 16:04
@dirk-thomas
Copy link
Member Author

dirk-thomas commented Aug 10, 2016

Fixed regression in e8f01f7.

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