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

Add ability to pass a block to render_wizard #145

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@gizotti
Copy link

gizotti commented Nov 24, 2014

  • Add ability to pass a block to render_wizard to be executed if the resource is saved successfully
  • Add capybara to gemfile so TravisCI tests won't fail
Add ability to pass a block to render_wizard to be executed if the re…
…source is saved successfully

add capybara to gemfile so tests won't fail on TravisCI

Add ability to pass a block to render_wizard to be executed if the resource is saved successfully
@schneems

This comment has been minimized.

Copy link
Owner

schneems commented Nov 24, 2014

What do you want to do with this? The current render_wizard is supposed to be as light weight as possible and closely mimic render except with the added redirect when skipping steps.

If the purpose is to see if the object will save, you should be doing that manually if @user.save, if the object is to see if @skip_to is set, maybe we should expose this in a better way.

Give me a real world use case of how you want to use this feature please.

@gizotti

This comment has been minimized.

Copy link
Author

gizotti commented Dec 15, 2014

@schneems

Sorry for taking so long here. Just had time to review my intentions with this change and how could I achieve it without the changes.
Ends up my changes are overkill and I can just use @user.save as you said. Was doing some work under pressure here and at the time my solution looked like the best.

I'll make sure that my future PR's are analysed better before sent and with better explanation.

Thanks for taking the time to look at it as well.

@schneems

This comment has been minimized.

Copy link
Owner

schneems commented Dec 15, 2014

Thanks for the response. Going to close this PR for now.

I'll make sure that my future PR's are analysed better before sent and with better explanation.

I appreciate the PR. I think all PRs whether they get merged or not are good times for you to dive into new code bases and for me to evaluate the current state of features and to imagine what could/should be in the future. A more thorough explanation can certainly help with discussion 😄

In general I try to state the problem (the current state of the world), explain the fix, and then explain how the world will be better after merging in the code. You can vaguely see that pattern here: rails/rails#13427

Not all my PRs get merged, but I always learn something from each of them. I consider them discussions with code. Anywhoo, i'm rambling. Thanks again for submitting the PR and thanks for using wicked ❤️ ❤️ ❤️ ❤️

@schneems schneems closed this Dec 15, 2014

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