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

fix over-eager namespace pruning when reparenting nodes #1333

Closed

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Aug 13, 2015

fixes #1332

@sgringwe
Copy link

sgringwe commented Nov 8, 2015

I would love to see this pull request merged as it affects us too, any update on this?

@larskanis larskanis added this to the 1.6.8 milestone Nov 27, 2015
@sgringwe
Copy link

@larskanis is there any update on this? we are still locked into @ccutrer 's version and would love to of course be able to get updates from newer versions

@larskanis
Copy link
Member

This PR sounds reasonable to me. It doesn't fully align JRuby and MRI implementations, but brings it closer together.

@flavorjones Could you please have a look at this?

@bradleybeddoes
Copy link

@larskanis @flavorjones any chance of this making it into 1.6.8? I'm also getting tripped up on this one.

@flavorjones
Copy link
Member

Yes, plan is to include in 1.6.8 final. Spent tonight going through old issues, will spend my next hack night finishing 1.6.8. Thanks for your patience.

@flavorjones
Copy link
Member

Apologies to just getting to this now -- the second test passes on master, can you help me understand why it's there?

Also, there are two conditionals being added to the ns check (both are NULL, or both are equal) and I'd like to make sure they're both being tested.

@bradleybeddoes
Copy link

What is the process here for assisting on a PR proposed by someone else?.

I am happy to try and assist resolve these questions but I don't want to impact/take any credit for the good work already done by @ccutrer.

@bradleybeddoes
Copy link

Further to the above comment if it is helpful in any way I've proposed #1444

@bradleybeddoes
Copy link

@flavorjones did you have any further thoughts on this PR or #1444 ?

Apologies for being a pest about this, it is just a really important fix for a new project I need to ship asap.

flavorjones added a commit that referenced this pull request May 30, 2016
and use same pattern in the existing test

related to #1332
flavorjones added a commit that referenced this pull request May 30, 2016
@flavorjones
Copy link
Member

I merged one of these tests and the fix into master manually and given you credit in the commit log.

I apologize I couldn't find a better way to keep your name on the commits.

Thank you so much!

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.

element in incorrect namespace after adding as a child
5 participants