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

Copy references #236

Merged
merged 2 commits into from
Jan 8, 2015
Merged

Copy references #236

merged 2 commits into from
Jan 8, 2015

Conversation

dantleech
Copy link
Contributor

NOTE: this PR is based on the numerical sorting PR (#226) which would need to be merged first.

If that PR is rejected I will need to rebase and update this PR, probably better to delay reviewing this until that is merged.

For changes see the second commit in this PR.

Depends

@dantleech
Copy link
Contributor Author

/cc @wachterjohannes @danrot

@lsmith77
Copy link
Member

lsmith77 commented Jan 5, 2015

please rebase

- New column "numerical_props" which is populated by numerical properies
  and which is used exlusively for ordering.
@dantleech dantleech force-pushed the copy_references branch 3 times, most recently from f4afc1e to 3a7cc80 Compare January 5, 2015 09:24
$referenceEls = $xpath->query('.//sv:property[@sv:type="reference"]');

foreach ($referenceEls as $referenceEl) {
if (array_key_exists($referenceEl->nodeValue, $resultSetUuids)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use isset here I think.

@dantleech
Copy link
Contributor Author

Rebased and updated the related phpcr-api-tests PR.

@lsmith77
Copy link
Member

lsmith77 commented Jan 5, 2015

can you rebase once more?

@dantleech
Copy link
Contributor Author

@lsmith77 the branch is already up-to-date here

@lsmith77
Copy link
Member

lsmith77 commented Jan 7, 2015

yes .. but if you rebase to master then the tests should pass

@dantleech
Copy link
Contributor Author

I will restart the tests .. but I can't rebase on master because there is nothing to rebase on.

@dantleech
Copy link
Contributor Author

All green :)

@dbu
Copy link
Member

dbu commented Jan 7, 2015

i wrote a comment on phpcr/phpcr-api-tests#148 - can you check that? would love to merge the tests first, then restart this build one more time, just to be sure.

@dbu
Copy link
Member

dbu commented Jan 7, 2015

merged the tests and restarted the build

@dantleech
Copy link
Contributor Author

Last test run failed with the random problem (cannot delete node /../idExample) @dbu mentioned a few days ago.

@dantleech
Copy link
Contributor Author

Build is green now however ...

lsmith77 added a commit that referenced this pull request Jan 8, 2015
@lsmith77 lsmith77 merged commit 842ca3a into master Jan 8, 2015
@lsmith77 lsmith77 deleted the copy_references branch January 8, 2015 09:29
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