Skip to content

All: Remove omittable exists variables #11775

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

Merged
merged 12 commits into from
Jan 10, 2023

Conversation

atorralba
Copy link
Contributor

Removes unneeded exists variables, as described in the QL-for-QL query created in #11770.

@atorralba atorralba force-pushed the atorralba/all/omittable-exists branch 2 times, most recently from 222a44f to 1789f9d Compare December 21, 2022 17:18
smowton
smowton previously approved these changes Dec 21, 2022
Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

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

Go lgtm

@atorralba atorralba force-pushed the atorralba/all/omittable-exists branch 2 times, most recently from 4a9c7db to dac25a5 Compare December 22, 2022 10:27
@atorralba atorralba added the no-change-note-required This PR does not need a change note label Dec 22, 2022
@atorralba atorralba marked this pull request as ready for review December 22, 2022 12:45
@atorralba atorralba requested a review from a team as a code owner December 22, 2022 12:45
@atorralba atorralba requested a review from a team December 22, 2022 12:45
@atorralba atorralba requested review from a team as code owners December 22, 2022 12:45
michaelnebel
michaelnebel previously approved these changes Jan 3, 2023
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

C# LGTM!

aibaars
aibaars previously approved these changes Jan 3, 2023
Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

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

:+1 for Ruby

yoff
yoff previously approved these changes Jan 4, 2023
Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

Python 👍

erik-krogh
erik-krogh previously approved these changes Jan 4, 2023
owen-mc
owen-mc previously approved these changes Jan 5, 2023
Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Approved for Go. I checked all of the types matched.

@atorralba atorralba force-pushed the atorralba/all/omittable-exists branch from 3d55c21 to f36e2cb Compare January 10, 2023 12:38
@atorralba atorralba force-pushed the atorralba/all/omittable-exists branch from f36e2cb to 5c388c5 Compare January 10, 2023 12:39
@atorralba
Copy link
Contributor Author

Thank you all for the reviews! 🙇

I had to rebase to fix conflicts. We're still missing a Java review, but due to the size of this PR and the high chances of conflicts appearing again soon, I feel inclined to just merge.

Could anyone re-approve?

@atorralba atorralba merged commit 72a11e7 into github:main Jan 10, 2023
@atorralba atorralba deleted the atorralba/all/omittable-exists branch January 10, 2023 15:07
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.

9 participants