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

Rename UniqueIdResolver to MetadataResolver #609

Merged

Conversation

dlurton
Copy link
Member

@dlurton dlurton commented May 23, 2022

#588 introduces UniqueIdResolver. While that name does accurately reflect what it currently does, in the future it will be expanded to cover much more than unique Ids. Therefore, I have decided that MetadataResolver is a much better name.

I would like to merge this to physical-plan-staging before #590 and #592 are merged so as to reduce PR churn. Thanks.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@dlurton dlurton requested a review from am357 May 23, 2022 17:37
@dlurton dlurton changed the base branch from main to physical-plan-staging May 23, 2022 17:37
@dlurton dlurton merged commit d2b6ede into physical-plan-staging May 23, 2022
@dlurton dlurton deleted the physical-plan-staging-metadata-resolver branch May 23, 2022 18:55
dlurton added a commit that referenced this pull request May 27, 2022
Merges the following PRs to `main`:

- #583 
- #584 
- #587 
- #588 
- #589 
- #609 
- #590 
- #592 

Also, this adds a TODO comment (ea40e4a, requested by @am357 in offline chat) and excludes `PlannerPipeline` from the new aggregate tests which were pulled in when merging from `main` and were not part of the other PRs (34025b3).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
RCHowell pushed a commit that referenced this pull request Jun 1, 2022
Merges the following PRs to `main`:

- #583 
- #584 
- #587 
- #588 
- #589 
- #609 
- #590 
- #592 

Also, this adds a TODO comment (ea40e4a, requested by @am357 in offline chat) and excludes `PlannerPipeline` from the new aggregate tests which were pulled in when merging from `main` and were not part of the other PRs (34025b3).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
RCHowell pushed a commit that referenced this pull request Jun 1, 2022
Merges the following PRs to `main`:

- #583 
- #584 
- #587 
- #588 
- #589 
- #609 
- #590 
- #592 

Also, this adds a TODO comment (ea40e4a, requested by @am357 in offline chat) and excludes `PlannerPipeline` from the new aggregate tests which were pulled in when merging from `main` and were not part of the other PRs (34025b3).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
RCHowell pushed a commit that referenced this pull request Jun 1, 2022
Merges the following PRs to `main`:

- #583 
- #584 
- #587 
- #588 
- #589 
- #609 
- #590 
- #592 

Also, this adds a TODO comment (ea40e4a, requested by @am357 in offline chat) and excludes `PlannerPipeline` from the new aggregate tests which were pulled in when merging from `main` and were not part of the other PRs (34025b3).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
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.

2 participants