Skip to content

chainable enrichments#58

Closed
sdepold wants to merge 1 commit intobrianc:masterfrom
sdepold:features/chainable_enrichments
Closed

chainable enrichments#58
sdepold wants to merge 1 commit intobrianc:masterfrom
sdepold:features/chainable_enrichments

Conversation

@sdepold
Copy link
Copy Markdown
Collaborator

@sdepold sdepold commented May 10, 2013

This PR targets #57. It only contains a test. I will see if I can get this implemented on my own. But as you know the logic better than me, it might be a good idea if you could give a quick answer if that is an easily solvable issue or not :)

@brianc
Copy link
Copy Markdown
Owner

brianc commented May 10, 2013

I definitely like the direction you're taking this. Having chainable queries is something I've been more and more interested in as it allows for query composition.

@sdepold
Copy link
Copy Markdown
Collaborator Author

sdepold commented May 10, 2013

do you think its easy to achieve or pretty complicated? I just looked into the code and got a bit confused ^^

@brianc
Copy link
Copy Markdown
Owner

brianc commented May 10, 2013

I think it's really easy to achieve actually. I am busy with work today, so might not get a chance, but could definitely have it in by the end of the weekend.

@brianc
Copy link
Copy Markdown
Owner

brianc commented May 10, 2013

also - sorry the code is confusing. 😬 I'll try to add comments to places as I modify them in the future. I usually rely on extensive unit testing more than comments but that's not the best way to go.

@sdepold
Copy link
Copy Markdown
Collaborator Author

sdepold commented May 10, 2013

cool :) looking forward. guess I will provide quite a lot of pull requests in the future. well at least if I get the code :)

@brianc
Copy link
Copy Markdown
Owner

brianc commented May 10, 2013

after a few pull requests are accepted I generally give out commit access if you're interested. If you'd rather just work through pull requests I'm cool with that too - pull requests only is my workflow at work. Sometimes it's easier to just git push origin master though.

@brianc
Copy link
Copy Markdown
Owner

brianc commented May 10, 2013

the only real rule is "all tests must pass and all new features must have tests"

@sdepold
Copy link
Copy Markdown
Collaborator Author

sdepold commented May 10, 2013

sounds like a plan :)

@HughePaul
Copy link
Copy Markdown
Collaborator

This sounds good. i'll keep an eye on this thread :)

@sdepold
Copy link
Copy Markdown
Collaborator Author

sdepold commented May 10, 2013

this can be closed, right?

@brianc
Copy link
Copy Markdown
Owner

brianc commented May 10, 2013

ya closing

@brianc brianc closed this May 10, 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.

3 participants