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

Compat work for Serialization #17671

Closed
danmoseley opened this issue Jun 21, 2016 · 7 comments
Closed

Compat work for Serialization #17671

danmoseley opened this issue Jun 21, 2016 · 7 comments

Comments

@danmoseley
Copy link
Member

Devise our plan for increasing compatibility with serialization, and break out the work.

@danmoseley
Copy link
Member Author

Also see #17073

@stephentoub stephentoub self-assigned this Jul 28, 2016
@stephentoub
Copy link
Member

stephentoub commented Jul 28, 2016

Remaining work:

@AlexGhiondea
Copy link
Contributor

@stephentoub do you know who is working on this?

@stephentoub
Copy link
Member

@stephentoub do you know who is working on this?

At the moment, no one, at least not with a specific serialiation focus. In many places, it's happening organically as part of folks bringing up other components and adding back members, at which point they're adding back the relevant ones here.

At one point I'd written/run a little tool to mark the types that were in corefx that needed to be made serializable. This is out-of-date now, and some of this has already been done, but it'll help to highlight many of the places that need serialization work in corefx:
stephentoub/corefx@898da04

Please feel free to pick it up and run with it. I'd shelved this as the big blocker was that many of the involved projects hadn't yet been upgraded to netstandard1.7, which makes it a much bigger task. @ericstj was looking at making a pass through the repo to address that, at which point this should be easier.

@danmoseley
Copy link
Member Author

Types missing ISerializable are tracked in https://github.com/dotnet/corefx/issues/12669

I'm wondering about [Serializable] attribute. That doesn't show up in the ref assys unfortunately.

@danmoseley
Copy link
Member Author

[Serializable] tracked by https://github.com/dotnet/corefx/issues/12725
Missing members tracked by https://github.com/dotnet/corefx/issues/12254

I don't think we need issues for CoreRT work. 80% CodeCoverage seems pretty good compared to much of CoreFX so probably not worth tracking.

@stephentoub any missing tests you had in mind? If not let's close this issue now

@stephentoub
Copy link
Member

[Serializable] tracked by dotnet/corefx#12725

Just note that for a type to be serializable, it must be [Serializable], but it may have additional serialization-related implementation, and some of that may not be visible by tracking missing members, e.g. there are [OnSerialized], [OnDeserialized], etc. attributes that can be used with [Serializable] and that wouldn't show up in a missing-member scan. We can still track the missing serialization work via [Serializable], but whoever does that work just needs to be aware that there may be things elsewhere in the class being attributed that needs to be fixed, too.

I don't think we need issues for CoreRT work.

Ok

any missing tests you had in mind?

There are two categories:

  1. Tests for BinaryFormatter
  2. Tests for specific [Serializable] implementations

I'm ok closing this for the first one, as I don't have any specific cases in mind, just that we're not hitting 20% of BinaryFormatter's implementation, which is much more than I'd like. For the second, I'm also ok not tracking it here, as long as whoever adds each [Serializable] implementation adds appropriate tests with it.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants