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

Have actions on the models? #492

Open
blueyed opened this issue Nov 9, 2017 · 6 comments
Open

Have actions on the models? #492

blueyed opened this issue Nov 9, 2017 · 6 comments

Comments

@blueyed
Copy link
Contributor

blueyed commented Nov 9, 2017

What is the reason to have the abstraction / indirection through pinax.stripe.actions, if most of it could be on the model directly, either as methods or classmethods?

@alexstewartja
Copy link

+1 for fat models, skinny views. I've decided to use pinax-stripe as loose inspiration for a more modern payments library, instead of hacking around poor design choices or struggling with the owner over obvious issues.

@blueyed
Copy link
Contributor Author

blueyed commented Nov 10, 2017

@alexstewartja
Is it available online somewhere (already)?

@paltman
Copy link
Contributor

paltman commented Nov 10, 2017

We've always had skinny views. We previously had fat models. But it was becoming hard to tie every bit of the API I wanted to provide to a particular model. So I made the decision to have skinny views AND skinny models and pull the business logic into its own subpackage.

It's mostly a design decision based on preference. There isn't really a technical reason why on models or in a business logic package is better over the other. That said, I felt that it might be easier to maintain as the library grew as I think the actions will far outnumber the models and it could make the models REALLY fat which can be hard to sort through (as we ran into previously), rather than having small modules with discreet functions.

@alexstewartja alexstewartja good luck to you.

@paltman
Copy link
Contributor

paltman commented Nov 10, 2017

@alexstewartja I went to try and remind myself of how you have struggled with the owner in the past as I was assuming you were talking about me. I wanted to make sure I hadn't been unpleasant to work with but I didn't find any reference to your github id in the repo other than the one pull request you sent that I merged. Maybe I'm searching badly. Could you remind me?

@paltman
Copy link
Contributor

paltman commented Nov 10, 2017

@blueyed i think you meant @alexstewartja (I made the same mistake and edited my comments)

@blueyed
Copy link
Contributor Author

blueyed commented Nov 16, 2017

Here we go (likely): #380 (comment)

@paltman paltman added this to the Rosie milestone Nov 23, 2017
@paltman paltman modified the milestones: Rosie, December 2017 Sprint Dec 1, 2017
@paltman paltman removed this from the December 2017 Sprint milestone Nov 25, 2021
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

No branches or pull requests

3 participants