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

Bug: Specifying a position with add_new_at: :top fails to insert at that position #220

Merged
merged 3 commits into from
Aug 23, 2016

Conversation

brendon
Copy link
Owner

@brendon brendon commented Aug 14, 2016

I've created a test that confirms the bug. Now I guess we figure out how to fix it.

I’ve changed the tests to cope with this change in behaviour. This will
potentially be a breaking change for people, though why someone would
be taking advantage of the ‘feature’ whereby specifying a `position` on
`add_new_at: :top` is ignored, is beyond me :)
@brendon
Copy link
Owner Author

brendon commented Aug 14, 2016

Hi fella's. I'd like some feedback on this one as it breaks currently tested behaviour, though the existing behaviour is weird to begin with. Let me know if you need any more details. I suppose we'd bump the minor version if we released this?

@swanandp
Copy link
Contributor

swanandp commented Aug 18, 2016

rbx failures are weird. I will look into them.

@brendon
Copy link
Owner Author

brendon commented Aug 18, 2016

Yes, they seem to be intermittent? Or perhaps I'm thinking of another time. Thanks for taking a look. I don't use Rubinius so I'm not sure what's going on :)

@brendon
Copy link
Owner Author

brendon commented Aug 18, 2016

Perhaps we're testing against an old version of rbx that is buggy? Is this one equivalent to mri2.1.0?

@brendon
Copy link
Owner Author

brendon commented Aug 22, 2016

Just closed and reopened this one to see if the rbx test failure was transient.

@brendon
Copy link
Owner Author

brendon commented Aug 22, 2016

Hi @swanandp, Rubinius seems to be a bit flaky with travis-ci. The latest test run just failed outright on installing the gems. Should we look at dropping support for that particular ruby franchise? It's certainly causing uncertainty in development.

@brendon
Copy link
Owner Author

brendon commented Aug 22, 2016

Scratch that, it seems it's to do with the extra gems we're specifying for rbx. I've removed them in a PR: #225 so we'll see how that builds. I'll then update this with that and see.

@brendon brendon closed this Aug 22, 2016
@brendon brendon reopened this Aug 22, 2016
@brendon
Copy link
Owner Author

brendon commented Aug 22, 2016

This is super frustrating. One pass and one failure, same code! :) And nothing major changed between versions :( Do we just go ahead and merge this @swanandp, and put it down to a bad travis stack?

@swanandp
Copy link
Contributor

This looks like an interesting problem to tack down, but let's go ahead an
merge this, since this is the right way to go.

On Tue, Aug 23, 2016 at 3:01 AM, Brendon Muir [email protected]
wrote:

This is super frustrating. One pass and one failure, same code! :) And
nothing major changed between versions :( Do we just go ahead and merge
this @swanandp https://github.com/swanandp, and put it down to a bad
travis stack?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#220 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFjGLW9dnQ0zbYDA_Dut-SLtmfryzd7ks5qihUhgaJpZM4Jj16J
.

@brendon
Copy link
Owner Author

brendon commented Aug 23, 2016

Yea, I've emailed the travis support team. See the Rails 4.1 test passed in the last run, then the 5.0 test failed! I think it's some instability on their end honestly. I'll merge this and do an 0.8.0 release.

@brendon brendon merged commit 7cd58c1 into master Aug 23, 2016
@brendon brendon deleted the add_new_at_top_bug branch August 23, 2016 22:25
@brendon brendon restored the add_new_at_top_bug branch August 23, 2016 22:26
@brendon
Copy link
Owner Author

brendon commented Aug 23, 2016

0.8.0 has been released and includes this change.

@brendon brendon deleted the add_new_at_top_bug branch March 14, 2017 01:22
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