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

Decide on changes for callbacks in response to new #save_draft method #55

Closed
chrisdpeters opened this issue Nov 11, 2016 · 2 comments
Closed
Assignees
Milestone

Comments

@chrisdpeters
Copy link
Collaborator

chrisdpeters commented Nov 11, 2016

I posed a question to @npafundi here in #47. I've since merged into master so I can work on some other features without an awful merge conflict.

Let's go ahead and have the discussion and track progress with callbacks in this issue.

Here is the question:

@npafundi I think that this is a good move for the API of this gem. It is a little confusing knowing when to call #draft_creation vs. #draft_update.

However, this could potentially affect how we do callbacks. If drafting behavior is moved into a single method, I'm not convinced that it would be clear to users of this gem when *_draft_creation vs. *_draft_update callbacks would be triggered. I think it's a little ambiguous: is before_draft_creation called when a draft record is being first created? Or is it when the item is created? See what I mean?

So I must ask, which of the callbacks are you using right now? Would you miss anything if I removed the ambiguous ones and created more general before_save_draft, after_save_draft, and around_save_draft callbacks?

@chrisdpeters chrisdpeters added this to the 0.6.0 milestone Nov 11, 2016
@chrisdpeters chrisdpeters changed the title Decide on changes for callbacks in response to #47 Decide on changes for callbacks in response to new #save_draft method Nov 11, 2016
@npafundi
Copy link
Contributor

Hey @chrisdpeters, thanks for reaching out! I think the changes in #47 make a lot of sense, so changing the callbacks accordingly sounds good.

I used draftsman heavily on a previous project, but I've since moved to a new company and I don't recall which of the callbacks we were using. Regardless, this seems like a good API change for draftsman, and creating less ambiguous callbacks would be a good improvement.

@chrisdpeters
Copy link
Collaborator Author

@npafundi Thanks for the feedback. Hope you're enjoying the new gig!

@chrisdpeters chrisdpeters self-assigned this Nov 12, 2016
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

2 participants