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

add possibility to use sql comments in the DQL syntax #8140

Merged
merged 3 commits into from
May 15, 2020

Conversation

philippe-levan
Copy link
Contributor

Hi,

Sometimes I want to add sql comments in DQL queries. This PR adds the possibility to use comments in DQL.

⚠️ this PR is for the branch 2.8.x

Best regards,
Philippe

@greg0ire
Copy link
Member

greg0ire commented May 14, 2020

You can run vendor/bin/phpcbf to fix the build. Also, please document this new feature.

@philippe-levan
Copy link
Contributor Author

Hi @greg0ire,

I didn't use phpcbf because it made to many changes on the file. I just fixed the coding style by hand. I believe this phpcbf should be run outside this feature PR in this case.

I added a documentation.

Thanks for your feedbacks,
Best regards,
Philippe

@greg0ire
Copy link
Member

I didn't use phpcbf because it made to many changes on the file.

You did well. I forgot we have this special behavior in this repo where we only fix CS for new code.

@greg0ire greg0ire requested a review from a team May 14, 2020 20:40
@guilhermeblanco
Copy link
Member

LGTM

@philippe-levan
Copy link
Contributor Author

LGTM

Sorry @guilhermeblanco, but what does mean LGTM ?

@greg0ire
Copy link
Member

https://www.urbandictionary.com/define.php?term=LGTM :) (I don't think it's the sixth meaning, don't worry)

@greg0ire greg0ire merged commit 0b305e5 into doctrine:2.8.x May 15, 2020
@greg0ire
Copy link
Member

Thanks @philippe-levan !

@philippe-levan
Copy link
Contributor Author

Thanks for the merge !

@greg0ire greg0ire self-assigned this May 15, 2020
@greg0ire greg0ire added this to the 2.8.0 milestone May 15, 2020
@philippe-levan
Copy link
Contributor Author

@greg0ire : do you want me to create the equivalent PR on master ?

@greg0ire
Copy link
Member

No don't worry, it will get merged up in time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants