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

Make FileDataReader support varying length ACTION records (#542) #550

Merged
merged 2 commits into from
Jun 8, 2012

Conversation

nremond
Copy link
Contributor

@nremond nremond commented Jun 8, 2012

I'm not sure I understood correctly the request as I don't see why we should remove the pattern matching ... tell me if I missed something ?

@slandelle
Copy link
Member

Regarding pattern matching, you were smarter than me, I hadn't thought of this solution. I'll just check the memory footprint.

Regarding the #543, the idea was to have FileDataReader collect all the files in the runId directory. I'll accept your request and revert back some changes.

Thanks,

Steph

slandelle pushed a commit that referenced this pull request Jun 8, 2012
Make FileDataReader support varying length ACTION records (#542)
@slandelle slandelle merged commit e49c008 into gatling:master Jun 8, 2012
@ghost ghost assigned slandelle Jun 8, 2012
@nremond
Copy link
Contributor Author

nremond commented Jun 8, 2012

My idea about #543 was to have a two classes :
RunDataReader(runId)
FileDataReader(filePath)

By splitting, it's also easier to unit test.

@slandelle
Copy link
Member

We could have a composite design, but I'm not ready to break the DataReader
signature as we might have other implementation that don't read from files,
but from other sources, such as a database.

2012/6/8 Nicolas Rémond <
reply@reply.github.com

My idea about #543 was to have a two classes :
RunDataReader(runId)
FileDataReader(filePath)

By splitting, it's also easier to unit test.


Reply to this email directly or view it on GitHub:
#550 (comment)

@nremond
Copy link
Contributor Author

nremond commented Jun 8, 2012

I get it. Are you on it?
I could do the modification right now

@slandelle
Copy link
Member

Yep, I'm on it.
Sorry if I stole your toy, I just wanted to finish this quickly so that
Stephen Kuesly can get a chance to work on #446 this week-end, and I can go
and get some sleep (it's been one hell of a week...).

I'll be thinking of more funny stuff you could play with.

Thanks for your help, it's very much appreciated.

Steph

2012/6/8 Nicolas Rémond <
reply@reply.github.com

I get it. Are you on it?
I could do the modification right now


Reply to this email directly or view it on GitHub:
#550 (comment)

@skuenzli
Copy link
Contributor

skuenzli commented Jun 9, 2012

Nice -- thank you for the new matching and tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants