Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

LeaveOpen ctor for CryptoStream#11587

Closed
bbowyersmyth wants to merge 2 commits into
dotnet:masterfrom
bbowyersmyth:CryptoStream_leaveOpen
Closed

LeaveOpen ctor for CryptoStream#11587
bbowyersmyth wants to merge 2 commits into
dotnet:masterfrom
bbowyersmyth:CryptoStream_leaveOpen

Conversation

@bbowyersmyth
Copy link
Copy Markdown
Contributor

private bool _finalBlockTransformed;
private SemaphoreSlim _lazyAsyncActiveSemaphore;

private bool _leaveOpen;
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.

Nit: readonly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@bartonjs
Copy link
Copy Markdown
Member

@bbowyersmyth this will require revving the contract version, and changing it to target netcoreapp1.1 (instead of netstandard1.x). There's an in-progress of that kind of change at #11272; but we don't have a good example where we've actually committed one yet. So we might need to hold off a couple more days while we get a reference example committed 😄

@stephentoub
Copy link
Copy Markdown
Member

@bartonjs, can we move this one forward now?

@bartonjs
Copy link
Copy Markdown
Member

We haven't merged a netstandard20 change into Cryptography.Primitives yet, so this either needs to wait a little longer to piggy back on #11959 or do all the netstandard1.7 related bumps in this change. (Or, a third option, just bump it in change by itself)

@bartonjs
Copy link
Copy Markdown
Member

bartonjs commented Oct 4, 2016

Okay, the change to restructure the ref assembly for S.S.C.Algorithms has gone in now. I don't know if this new member should go in netstandard1.7 or needs to be forked off as netcoreapp11 (or netcoreapp12?). Do you happen to know, @stephentoub? 😄

@Petermarcu
Copy link
Copy Markdown
Member

@ericstj , can you confirm? In general, things going into master right now go into netcoreapp1.2.

@ericstj
Copy link
Copy Markdown
Member

ericstj commented Oct 10, 2016

@ericstj , can you confirm? In general, things going into master right now go into netcoreapp1.2.

If the API already exists in desktop it's goes into netstandard1.7 (until we get a netstandard2.0 mapped by NuGet).

If the API is brand-new it should go into netcoreapp*.

@ericstj
Copy link
Copy Markdown
Member

ericstj commented Oct 12, 2016

To be clear, things in master right now are targeting netstandard1.7 / netcoreapp1.1 as a stopgap until we move them to netstandard2.0 / netcoreapp1.2.

@karelz
Copy link
Copy Markdown
Member

karelz commented Oct 15, 2016

#11959 is now merged. What should happen with this PR?
cc: @steveharter

@bartonjs
Copy link
Copy Markdown
Member

I'm cherry-picking the current commits (though squashed into one) and adding the contract change, and will likely replace this PR shortly.

@bartonjs
Copy link
Copy Markdown
Member

Solved merge conflict and added contract work in #12718, closing this PR as it has been replaced.

@bartonjs bartonjs closed this Oct 17, 2016
@bartonjs bartonjs removed their assignment Nov 15, 2016
@karelz karelz modified the milestone: 1.2.0 Dec 3, 2016
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.

8 participants