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

Design capability APIs #24259

Closed
terrajobst opened this issue Nov 27, 2017 · 6 comments
Closed

Design capability APIs #24259

terrajobst opened this issue Nov 27, 2017 · 6 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Milestone

Comments

@terrajobst
Copy link
Member

We need to list & drive a design of capability APIs as explained here:

https://github.com/dotnet/designs/blob/master/accepted/compat-pack/compat-pack.md#non-goals

Instead of writing code like this:

private static string GetLoggingPath()
{
    if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
    {
        using (var key = Registry.CurrentUser.OpenSubKey(@"Software\Fabrikam\AssetManagement"))
        {
            if (key?.GetValue("LoggingDirectoryPath") is string configuredPath)
                return configuredPath;
        }
    }

    var appDataPath = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
    return Path.Combine(appDataPath, "Fabrikam", "AssetManagement", "Logging");
}

we should be promoting code like this:

private static string GetLoggingPath()
{
    if (Registry.IsSupported)
    {
        using (var key = Registry.CurrentUser.OpenSubKey(@"Software\Fabrikam\AssetManagement"))
        {
            if (key?.GetValue("LoggingDirectoryPath") is string configuredPath)
                return configuredPath;
        }
    }

    var appDataPath = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
    return Path.Combine(appDataPath, "Fabrikam", "AssetManagement", "Logging");
}
@terrajobst terrajobst self-assigned this Nov 27, 2017
@JonHanna
Copy link
Contributor

Perhaps relevant: #17973 Here people can "do the thing", but impacts of implementations might mean they don't want to.

@danmoseley
Copy link
Member

Note that your first example has also the problem that it throws PNSE on UWP.

We may want to prefer a loosely typed system like IsSupported("Registry") so that we don't need to update all platforms before a new value becomes useable. AppContext/quirks seem to do this.

@terrajobst
Copy link
Member Author

We may want to prefer a loosely typed system like IsSupported("Registry") so that we don't need to update all platforms before a new value becomes useable. AppContext/quirks seem to do this.

Yes, but this would require a centralized place, which I don't think we want. The registry is a great example: it doesn't ship in-box with UWP or .NET Core -- it's a NuGet package. If the support inside the package changes, we shouldn't be forced to update the runtime for that.

Having a centralized override for quirking seems desirable but I don't see how this would work. If the library returned false from its capability API overriding it to true doesn't make more stuff work IMHO. And the reverse seems nonsensical to me.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@joperezr joperezr removed the untriaged New issue has not been triaged by the area owner label Jul 6, 2020
@joperezr joperezr modified the milestones: 5.0.0, Future Jul 6, 2020
@stephentoub
Copy link
Member

@terrajobst, are you still working on this or can it be closed?

@dakersnar
Copy link
Contributor

@terrajobst bump on the above question

@terrajobst
Copy link
Member Author

Not at this point, no

@terrajobst terrajobst closed this as not planned Won't fix, can't repro, duplicate, stale Dec 14, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Projects
None yet
Development

No branches or pull requests

9 participants