Skip to content
This repository has been archived by the owner on Oct 2, 2019. It is now read-only.

Extract Query objects from Reporter class #928

Merged
merged 3 commits into from Feb 5, 2016
Merged

Conversation

jessieay
Copy link
Contributor

@jessieay jessieay commented Feb 5, 2016

@jessieay jessieay force-pushed the jy-sql-funtimes branch 4 times, most recently from 39529ec to 524153e Compare February 5, 2016 04:47
@jessieay
Copy link
Contributor Author

jessieay commented Feb 5, 2016

argh this code climate error is due to duplication and I am not sure if / how best to fix.

@jessieay jessieay closed this Feb 5, 2016
@jessieay jessieay reopened this Feb 5, 2016
@pkarman
Copy link
Contributor

pkarman commented Feb 5, 2016

I like the code refactor here to split out the queries into their own classes.

As far as going further to rewrite in raw SQL, I've commented on that Trello card that I don't consider it worth the effort.

@jessieay
Copy link
Contributor Author

jessieay commented Feb 5, 2016

@pkarman agreed. replied to your comment!

* Cannot join on polymorphic
@pkarman
Copy link
Contributor

pkarman commented Feb 5, 2016

@jessieay test failures?

I would ignore that code duplication error. It's not a duplicate. Methinks codeclimate algorithm needs some tweeking.

@jessieay
Copy link
Contributor Author

jessieay commented Feb 5, 2016

@pkarman woops, should pass now.

Agreed on duplication!

pkarman added a commit that referenced this pull request Feb 5, 2016
Extract Query objects from Reporter class
@pkarman pkarman merged commit 41c7f36 into master Feb 5, 2016
@pkarman pkarman deleted the jy-sql-funtimes branch February 5, 2016 22:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants