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

state: convert nodes.ID to the new pattern of functional indexers #9797

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Feb 19, 2021

Branched from #9796

@dnephin dnephin requested a review from a team February 19, 2021 23:21
In preparation for adding other identifiers to the index.
@dnephin dnephin force-pushed the dnephin/state-index-node-id branch from 5130b42 to a4e68e3 Compare March 5, 2021 17:30
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 5, 2021 17:30 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 5, 2021 17:31 Inactive
Copy link
Member

@banks banks left a comment

Choose a reason for hiding this comment

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

I like this functional approach a lot.

I was a little hesitant about the Query object at first but actually I think it's a clear solution to encapsulating optional fields between Ent and OSS and it reads well in the code 👍 .

Base automatically changed from dnephin/state-cleanup-catalog-index-oss to master March 10, 2021 17:20
@dnephin dnephin merged commit 9d924a8 into master Mar 10, 2021
@dnephin dnephin deleted the dnephin/state-index-node-id branch March 10, 2021 22:34
@hashicorp-ci
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/335927.

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