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

Confirmation tweaks #2545

Closed
wants to merge 2 commits into from

Conversation

markturner
Copy link

Resolves issue #2544

@josevalim
Copy link
Contributor

@markturner, this looks great, thank you! Could you please take a look at the build? This pull request is breaking it.

@markturner
Copy link
Author

Oops, sorry! I've fixed that now, except for just 2 remaining tests where I'm not quite sure what's happening.

I'll take another look over the weekend unless you or someone else can figure it out.

@markturner
Copy link
Author

Sorry, I wasn't able to figure it out and I'm not going to be able to put anymore time into it. It's probably obvious to someone else but my testing knowledge is pretty much all rspec! Would be great if someone with more of a handle on this than me could get these last two tests to pass.

@josevalim
Copy link
Contributor

Thanks @markturner, don't worry. We will get around to it and merge your fixes in.

@CraigSnowden
Copy link

Looks like this has already been integrated?

@josevalim
Copy link
Contributor

No, I don't think it has yet.

@ghost
Copy link

ghost commented Feb 17, 2015

Is this still relevant?

@ulissesalmeida
Copy link
Contributor

@markturner Cold you please rebase your branch? Thanks.

@ulissesalmeida ulissesalmeida added this to the 4.1.0 milestone Apr 11, 2016
@lucasmazza lucasmazza modified the milestone: 4.1.0 Apr 29, 2016
@tegon
Copy link
Member

tegon commented Dec 13, 2017

I'm closing this since it has been here for a long time and probably will be too hard to rebase it.
If you find that this can still be relevant, feel free to open a new pull request with a similar implementation.

Thank you!

@tegon tegon closed this Dec 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

6 participants