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

Simplify fixture generation and add customization hooks #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jordan-brough
Copy link
Contributor

@rdy putting this out there to get your thoughts on it. We've been using this and it's been really handy for a couple custom things. Lmk what you think and I can add some tests if it seems OK.

  • Always use ActiveRecord instead of only when we happen to be able
    to constantize the name.
  • Add select_scope_proc for custom record selection
  • Add hashize_record_proc for custom serialization
  • Use attributes_before_type_cast

@rdy
Copy link
Owner

rdy commented Apr 3, 2016

Let me look this over, I've been a little busy this week but I will make it a priority. I plan on fixing the Travis build issues first.

@rdy
Copy link
Owner

rdy commented Apr 4, 2016

Please rebase the branch of master to fix the Travis issues

- Always use ActiveRecord instead of only when we happen to be able
  to constantize the name.
- Add select_scope_proc for custom record selection
- Add hashize_record_proc for custom serialization
- Use attributes_before_type_cast
@jordan-brough
Copy link
Contributor Author

@rdy I've rebased and the tests are now green. Let me know what you think in general and if it seems OK I can add tests to cover the new functionality.

@jordan-brough
Copy link
Contributor Author

@rdy ping :)

@rdy
Copy link
Owner

rdy commented Apr 28, 2016

I'll try to merge this by the weekend. If you don't mind adding tests for the new functionality I would be more inclined to merge the request quickly.

@jordan-brough
Copy link
Contributor Author

Super! I'll add some tests in the next few days and then ping you for a merge. I just wanted to make sure you were OK with the idea before I spent the extra effort. Thanks!

@thewoolleyman
Copy link
Collaborator

@jordan-brough If you add tests, we can look at testing and merging this.

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

Successfully merging this pull request may close these issues.

None yet

3 participants