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

Use new factory definition syntax #110

Merged
merged 2 commits into from
Mar 26, 2016
Merged

Conversation

paulcsmith
Copy link
Contributor

Pros

  • Can use symbol view/ctags to jump to the factories more easily
  • Can add helper functions underneath the factory they belong to
  • IMO it's easier to scan your factory module for a particular factory
  • Easier to type :)
  • If/when callbacks are introduced, this will allow you to put the
    callback functions near the factory

Cons

  • A little more magic since you still call the functions with atoms,
    e.g.build(:user)

Closes #101

Pros

* Can use symbol view/ctags to jump to the factories more easily
* Can add helper functions underneath the factory they belong to
* Easier to type :)
* If/when callbacks are introduced, this will allow you to put the
  callback functions near the factory

Cons

* A little more *magic* since you still call the functions with atoms,
  e.g.`build(:user)`
@jsteiner
Copy link
Contributor

I'm down with this, mainly for defining helper functions and callbacks near the function.

I'm not sure how often ctags would be useful since it would be rare to use the function directly.

@paulcsmith
Copy link
Contributor Author

Good point. Ctags are probably useless here, but a lot of editors have "Jump to Definition/Symbol Views" where you can jump to a function. Right now they're all called factory so this should make it easier for people using those editors (I hope :D)

I'll go ahead and merge this in. Thanks for the review!

@paulcsmith paulcsmith merged commit 384fbec into master Mar 26, 2016
@paulcsmith paulcsmith deleted the ps-change-factory-definition branch March 26, 2016 23:01
@paulcsmith paulcsmith mentioned this pull request Jun 24, 2016
@quolpr
Copy link

quolpr commented Apr 4, 2018

The regex that can help you to migrate to new factory version:
Replace def factory\(:(.*?)\) with def $1_factory

Can be done in atom/vscode

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