Skip to content

Address feedback on ARM heuristic LRO helper implementation and tests#4510

Merged
heaths merged 3 commits into
copilot/add-heuristic-lro-helperfrom
copilot/wip-address-feedback-on-arm-heuristic-lro-helper-i
Jun 2, 2026
Merged

Address feedback on ARM heuristic LRO helper implementation and tests#4510
heaths merged 3 commits into
copilot/add-heuristic-lro-helperfrom
copilot/wip-address-feedback-on-arm-heuristic-lro-helper-i

Conversation

Copilot AI commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for the feedback on #4508. I've created this new PR, which merges into #4508, to address your comment. I will work on the changes and keep this PR's description up to date as I make progress.

Original PR: #4508
Triggering reviews:

@copilot expand on these monikers better, like "async" should mention the azure-asyncoperation header.

@copilot since we clone the initial_request above as well as its URL, I don't think we need to clone resource_url here yet again. Can't we just initial_request.url().clone() here?

@copilot make sure we cover all test cases for async, body, and loc directories for *_test.go files under https://github.com/Azure/azure-sdk-for-go/tree/main/sdk/azcore/internal/pollers.

@copilot this field shouldn't be necessary. Undo changes related to this state field and I'll handle that in a separate PR.

Copilot AI and others added 2 commits June 2, 2026 01:22
- Revert `state` field from `PollerOptions` (comment #3338002639)
- Remove `new_poller_can_resume_from_state` test (depended on removed field)
- Fix `resource_url` clone: use `initial_request.url().clone()` inside
  the closure instead of cloning a captured `resource_url` variable
  (comment #3337973698)
- Expand doc comment on `new_poller` to describe each polling mode with
  its associated header name (comment #3337968630)
- Handle empty-body (204 No Content) responses by deserialising `{}`
  so status falls through to the HTTP status-code check
- Fall back to `resource_url` in the Succeeded arm so synchronous
  completions work when there is no prior final_link
- Add six new test cases ported from the Go SDK async/body/loc pollers
  (comment #3337983148): no-location async, synchronous completion,
  async/body/location failed, body/location 204 No Content
- Run cargo fmt and cargo clippy (comment #4405613982)

Co-authored-by: heaths <1532486+heaths@users.noreply.github.com>
… completion test

Co-authored-by: heaths <1532486+heaths@users.noreply.github.com>
Copilot finished work on behalf of heaths June 2, 2026 01:28
Copilot AI requested a review from heaths June 2, 2026 01:28
@heaths heaths marked this pull request as ready for review June 2, 2026 01:37
@heaths heaths changed the title [WIP] [WIP] Address feedback on ARM heuristic LRO helper implementation and tests Address feedback on ARM heuristic LRO helper implementation and tests Jun 2, 2026

@heaths heaths left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a terrible way to review changes I requested to a PR.

@heaths heaths merged commit a1c5e9b into copilot/add-heuristic-lro-helper Jun 2, 2026
4 checks passed
@heaths heaths deleted the copilot/wip-address-feedback-on-arm-heuristic-lro-helper-i branch June 2, 2026 01:38
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