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 "pebble" mode #35

Merged
merged 6 commits into from
Aug 6, 2013
Merged

Add "pebble" mode #35

merged 6 commits into from
Aug 6, 2013

Conversation

gvc
Copy link
Contributor

@gvc gvc commented Jul 27, 2013

Closes #29.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 16fde1a on gvc:add_pebble_object into 0869d4a on avdi:master.

@avdi
Copy link
Owner

avdi commented Jul 28, 2013

This makes me happy!

2 things, if you don't mind:

  1. Make it into a command object instead of method on the builder.
  2. I'm not a big fan of putting expectations on Kernel in tests. I'm also not sure why we need p here instead of puts, since we don't need to inspect the interpolated string. Can you update this to either a) capture stdout and test the captured tests; or b) optionally dependency-inject an "out" role and use that role to do the output? If you're not clear on how to do these I can take a crack at it.

Thanks!

@gvc
Copy link
Contributor Author

gvc commented Jul 29, 2013

Great @avdi, thank you for your input. I'll try and tackle these questions this week. If I get stuck somewhere I'll let you know!

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8b615cb on gvc:add_pebble_object into 0869d4a on avdi:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling b2c68a0 on gvc:add_pebble_object into 0869d4a on avdi:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 6ec840a on gvc:add_pebble_object into 0869d4a on avdi:master.

@julianalucena
Copy link
Contributor

Hey, @avdi. Could you look at it again, please?

@avdi
Copy link
Owner

avdi commented Aug 6, 2013

Rockin'!

avdi pushed a commit that referenced this pull request Aug 6, 2013
@avdi avdi merged commit 5131062 into avdi:master Aug 6, 2013
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.

Add "pebble" mode
4 participants