Skip to content

Use less Opaleye internals in aggregation#204

Merged
shane-circuithub merged 2 commits intocircuithub:masterfrom
tomjaguarpaw:aggregator
Nov 30, 2022
Merged

Use less Opaleye internals in aggregation#204
shane-circuithub merged 2 commits intocircuithub:masterfrom
tomjaguarpaw:aggregator

Conversation

@tomjaguarpaw
Copy link
Copy Markdown
Contributor

Would it be OK to do this, to depend on less Opaleye internals?

@tomjaguarpaw
Copy link
Copy Markdown
Contributor Author

Hey folks, any thoughts on this? This would allow me to rearrange the Opaleye aggregate internals, which are in need of clarification.

@ocharles
Copy link
Copy Markdown
Contributor

@shane-circuithub Could you take a look at this?

@shane-circuithub
Copy link
Copy Markdown
Contributor

I'm generally in favour of this. I know we currently don't use the ordering field of our Aggregator type, but we have used it in the past. Is there an equivalent of distinctAggregator for adding an ordering @tomjaguarpaw ?

@tomjaguarpaw
Copy link
Copy Markdown
Contributor Author

Are you looking for orderAggregate?

@shane-circuithub
Copy link
Copy Markdown
Contributor

Yep, that looks like what I mean. In that case I don't see any downside to this.

@shane-circuithub shane-circuithub merged commit 6c038ec into circuithub:master Nov 30, 2022
@ocharles
Copy link
Copy Markdown
Contributor

Thanks @shane-circuithub!

@tomjaguarpaw tomjaguarpaw deleted the aggregator branch December 1, 2022 08:11
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