Skip to content

qom always has a selector name #86

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 25, 2013
Merged

qom always has a selector name #86

merged 2 commits into from
Sep 25, 2013

Conversation

dbu
Copy link
Member

@dbu dbu commented Jul 25, 2013

adjust phpcr-utils to phpcr/phpcr#68

dbu
dbu reviewed Jul 25, 2013
View reviewed changes
src/PHPCR/Util/QOM/Sql2ToQomQueryConverter.php
default:
// Exit loop for debugging
break(2);
throw new InvalidQueryException('Expected end of query, got ' . $this->scanner->lookupNextToken() . ' in ' . $this->sql2);
Copy link
Member Author

Choose a reason for hiding this comment

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

this was never cleaned up

@lsmith77
Copy link
Member

kind of strange that the tests are passing .. but i guess we do not have any query tests in here.

@dbu
Copy link
Member Author

dbu commented Jul 25, 2013

yeah we need a qom factory implementation to do that seriously. so the
tests are all in phpcr-api-tests (and they succeed when using all 4 PR
together)

@lsmith77
Copy link
Member

so lets merge?

@dbu
Copy link
Member Author

dbu commented Jul 25, 2013

this needs to be coordinated with the other 3 PR on phpcr and phpcr-api-tests and jackalope - did you look into them as well?

@lsmith77
Copy link
Member

the composer.json should also be updated even if we do not yet have the necessary tag

@dbu
Copy link
Member Author

dbu commented Jul 26, 2013

i updated composer.json and merged phpcr already as we depend on an exact version there.

this PR needs to be synchronized with jackalope as we depend on ~1.0.0-beta10

@lsmith77 lsmith77 closed this Aug 13, 2013
@lsmith77 lsmith77 reopened this Aug 13, 2013
@dbu dbu mentioned this pull request Aug 14, 2013
Merged
@dbu
Copy link
Member Author

dbu commented Aug 28, 2013

we also need to fix QueryBuilder::select and addSelect
https://github.com/phpcr/phpcr-utils/blob/master/src/PHPCR/Util/QOM/QueryBuilder.php#L377

lsmith77 added a commit that referenced this pull request Sep 25, 2013
@lsmith77
Merge pull request #86 from phpcr/no-empty-selector
11cac16 
qom always has a selector name
@lsmith77 lsmith77 merged commit 11cac16 into master Sep 25, 2013
@lsmith77 lsmith77 deleted the no-empty-selector branch September 25, 2013 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dbu @lsmith77