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

TypedText - Added versions of dailyPrefixSuffix for TSV and CSV sources. #1627

Merged
merged 2 commits into from
Dec 5, 2016
Merged

TypedText - Added versions of dailyPrefixSuffix for TSV and CSV sources. #1627

merged 2 commits into from
Dec 5, 2016

Conversation

cavorite
Copy link
Contributor

@cavorite cavorite commented Dec 1, 2016

TypedText already has a method for reading OSV sources with a path built from a prefix and a suffix. This PR adds 2 variants of that method for reading TSV and CSV sources.

@johnynek
Copy link
Collaborator

johnynek commented Dec 1, 2016

Can we have one private method parameterized on the separator and share code across all three?

@johnynek
Copy link
Collaborator

johnynek commented Dec 2, 2016

PS: thanks for the contribution!!

@cavorite
Copy link
Contributor Author

cavorite commented Dec 3, 2016

I added a private function for each of the variants of the methods (hourly, daily with prefix and daily with prefix and suffix).

@johnynek
Copy link
Collaborator

johnynek commented Dec 3, 2016

👍 merge when green

@cavorite
Copy link
Contributor Author

cavorite commented Dec 5, 2016

I currently don't have write permissions, @johnynek. Could you merge the PR for me?

@johnynek johnynek merged commit 845c33f into twitter:develop Dec 5, 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