Skip to content

Use buffer.Length as the source of truth for ReadDirectoryChanges' nBufferLength #73970

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 16, 2022
Merged

Use buffer.Length as the source of truth for ReadDirectoryChanges' nBufferLength #73970

merged 1 commit into from
Aug 16, 2022

Conversation

jozkee
Copy link
Member

@jozkee jozkee commented Aug 15, 2022

No description provided.

@jozkee jozkee added this to the 7.0.0 milestone Aug 15, 2022
@jozkee jozkee requested a review from carlossanlop August 15, 2022 21:24
@jozkee jozkee self-assigned this Aug 15, 2022
@ghost
Copy link

ghost commented Aug 15, 2022

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: Jozkee
Assignees: Jozkee
Labels:

area-System.IO
Milestone: 7.0.0

danmoseley
danmoseley approved these changes Aug 15, 2022
View reviewed changes
Copy link
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

LGTM. I suppose in theory this could have a test that attempts to force the race, but if we were writing this code fresh, I don't think we'd write such a test, and the fix is self evidently how we should have written it.

@danmoseley
Copy link
Member

failures so far are the prevalent issue right now... "failed to upload results"

adamsitnik
adamsitnik approved these changes Aug 16, 2022
View reviewed changes
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM

@danmoseley danmoseley closed this Aug 16, 2022
@danmoseley danmoseley reopened this Aug 16, 2022
@jozkee jozkee merged commit 9a72a29 into dotnet:main Aug 16, 2022
@jozkee jozkee modified the milestones: 7.0.0, 8.0.0 Aug 16, 2022
@jozkee jozkee deleted the fsw_bufferlength branch August 16, 2022 19:44
@jozkee
Copy link
Member Author

jozkee commented Aug 16, 2022

/backport to release/7.0-rc1

@github-actions
Copy link
Contributor

Started backporting to release/7.0-rc1: https://github.com/dotnet/runtime/actions/runs/2870660998

@github-actions github-actions bot mentioned this pull request Aug 16, 2022
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stephentoub stephentoub stephentoub left review comments

@carlossanlop carlossanlop carlossanlop approved these changes

@adamsitnik adamsitnik adamsitnik approved these changes

@danmoseley danmoseley danmoseley approved these changes

Assignees

@jozkee jozkee

Labels
area-System.IO
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@jozkee @danmoseley @carlossanlop @stephentoub @adamsitnik