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

[Netfx bug] MaxLengthAttribute and MinLengthAttribute don't support ICollection.Count #21101

Closed
hughbe opened this issue Apr 14, 2017 · 12 comments

Comments

@hughbe
Copy link
Contributor

hughbe commented Apr 14, 2017

Issue requested by @ajcvickers. We should tag this one as netfx-port-consider and close it.

The following code throws an InvalidCastException . .NET Core added support for this feature. See dotnet/corefx#2650

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;

namespace MyNamespace
{
    public class MyDataModel
    {
        [MinLength(1)]
        public ICollection<string> MySet { get; set; }
    }
}
@danmoseley
Copy link
Member

Clsoing as requested by @ajcvickers

@ajcvickers
Copy link
Member

@danmosemsft I don't think I requested this to be closed...

@stephentoub stephentoub reopened this Apr 14, 2017
@danmoseley
Copy link
Member

I apologize - I misread.

@hughbe
Copy link
Contributor Author

hughbe commented Apr 15, 2017

I thought the Idea with these kinds of issues is that we mark them as netfx port consider then close them. Am I wrong?

@karelz
Copy link
Member

karelz commented Apr 15, 2017

Correct, if this is tracking change only in Desktop, then we should close it -- it will be mirrored into VSO bug database (even closed), so that we will consider it one day when we have time to focus on porting .NET Core goodness into Desktop.

Closing again. @ajcvickers if there is something that has to happen in CoreFX repo, feel free to reopen (with details).

@karelz karelz closed this as completed Apr 15, 2017
@ajcvickers
Copy link
Member

@karelz So does that mean that any changes that have already gone in to core, even if they were done a couple of years ago when compat was not considered that important, are automatically okay? If so, that makes our job much easier because it makes core basically the baseline against which we measure everything. It means that we will never consider changing core to work more like desktop, only the other way round.

@karelz
Copy link
Member

karelz commented Apr 16, 2017

No, we still need assess compat with .NET Framework and decide (by area owners) what is acceptable and what is not.
If you make .NET Core different from .NET Framework, you have to be either

  1. Convinced you can make the same change in Desktop eventually and it will pass compat there (the bar is high!), or
  2. Convinced that the difference is the right thing to do and that the impact on library authors and app authors won't be large and there is non-trivial value in the "new behavior".
    • Don't forget that especially library authors won't test their code on all platforms (high cost), so you need to asses how likely it is they are going to run into the differences without knowing, shipping them to their customers and then eventually dealing with the fallout. You can check with .NET AppCompat team and experts on the topic.
    • The good news is that we will have tooling hinting what's incompatible/obsolete (see prototype), but we still need to assess the impact on library/app authors who hit it, and how easy it is for them to workaround the differences (ideally auto-fix).

I understand that this is rarely black or white. If you have further questions, feel free to start internal discussion / meeting to clarify things ... (also note that I am not the only authority on the subject :))

@ajcvickers
Copy link
Member

@karelz How do we do that if the issues opened for us to do such analysis are closed before we have had a chance to do the analysis?

@karelz
Copy link
Member

karelz commented Apr 17, 2017

@ajcvickers if you still need to assess this particular breaking change in 2.0, then it totally make sense to keep the issue open.
Based on the bug title "netfx bug" and the original description "We should tag this one as netfx-port-consider and close it.", I assumed, this is only tracking something in .NET Framework and won't impact .NET Core 2.0. If that's not true, feel free to reopen.

My apologies if you feel I overstepped here. The netfx-port-consider itself is tricky process and not very well-known, so I am trying to help area owners with it. I'll be happy to chat about any additional feedback you might have on CoreFX bug management and process involved with it.

@weitzhandler
Copy link
Contributor

weitzhandler commented Aug 22, 2017

Will the full .NET framework ever support ICollection.Count?

And BTW, related: dotnet/corefx#23461.

@karelz
Copy link
Member

karelz commented Aug 22, 2017

It is marked as netfx-port-consider, so it will be considered in some future.

@karelz
Copy link
Member

karelz commented Aug 22, 2017

Let's not mix different bug discussions. That one is covered in your linked issue.

@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 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants