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

Add in support for UUID substitution #7

Closed
wants to merge 13 commits into from
Closed

Conversation

gtrevg
Copy link

@gtrevg gtrevg commented Mar 26, 2014

Updates to cover #6

@sonots
Copy link
Owner

sonots commented Mar 26, 2014

Thanks, but could you fix followings?

  1. Travis is failing. It seems you forgot to add uuidtools gem on gemspec file. See https://github.com/tagomoris/fluent-mixin-config-placeholders/blob/master/fluent-mixin-config-placeholders.gemspec#L18
  2. Need tests for uuid placeholders
  3. Need to only generate the needed UUID if that variable were passed in, otherwise, this codes make speed slower, and affect people who do not need to use UUID.

@gtrevg
Copy link
Author

gtrevg commented Mar 26, 2014

To run / update unit tests, would that be modifying the spec/out_record_reformer_spec.rb file?

Thanks

@sonots
Copy link
Owner

sonots commented Mar 26, 2014

@gtrevg Yes

@gtrevg
Copy link
Author

gtrevg commented Mar 26, 2014

Will do.

@sonots
Copy link
Owner

sonots commented May 7, 2014

Let me close this pull request once because it looks there is no progress. The issue is kept open.

@sonots sonots closed this May 7, 2014
@gtrevg
Copy link
Author

gtrevg commented May 7, 2014

No problem. I think you've also made some other changes, so I'll just fork a fresh copy and will do a pull request of a more concise update.

Thanks,

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