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

Simplify ResourcePublisher #1346

Merged
merged 4 commits into from
Dec 13, 2023

Conversation

drewnoakes
Copy link
Member

@drewnoakes drewnoakes commented Dec 12, 2023

We publish updates to resources via IAsyncEnumerable<ResourceChange>. The previous code defined a custom enumerable/enumerator for this, but it's possible to use a generator function instead and reduce the amount of code a bit.

Microsoft Reviewers: Open in CodeFlow

We publish updates to resources via `IAsyncEnumerable<ResourceChange>`. The previous code defined a custom enumerable/enumerator for this, but it's possible to use a generator function instead and reduce the amount of code a bit.
Copy link
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

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

Changes look fine.

Is there test coverage of this area? If not, it would be good to have some to verify it works the same before/after.

@drewnoakes
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

It's more consistent with the other enum member, named Upsert (which should otherwise be Upserted).
@drewnoakes
Copy link
Member Author

@JamesNK I added ResourcePublisherTests.

@drewnoakes drewnoakes merged commit 2441c9b into dotnet:main Dec 13, 2023
8 checks passed
@drewnoakes drewnoakes deleted the simplify-resource-publisher branch December 13, 2023 00:41
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants