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.

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

Reconsider naming of ObsoletedInOSPlatformAttribute #72970

Closed
terrajobst opened this issue Jul 27, 2022 · 2 comments · Fixed by #73600
Closed

Reconsider naming of ObsoletedInOSPlatformAttribute #72970

terrajobst opened this issue Jul 27, 2022 · 2 comments · Fixed by #73600
Assignees
@buyaa-n
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime blocking Marks issues that we want to fast track in order to unblock other important work
Milestone
7.0.0

Comments

@terrajobst
Copy link
Member

terrajobst commented Jul 27, 2022 •
edited
Loading

From @jeffhandley delivered by @buyaa-n:

Immo we wonder why the ObsoletedInOSPlatformAttribute has In where others just SupportedOSPlatformAttribute and UnsupportedOSPlatformAttribute, our messaging structure is like

This call site is reachable on {platforms list}. 'The.API' is supported on: {platforms list}

Then the Obsoleted attribute as In but the message will have on:

This call site is reachable on {platforms list}. 'The.API' is obsoleted on: {platforms list}

We might want to change the attribute to similar ObsoletedOSPlatformAttribute or there is specific reason of having In? 
@terrajobst terrajobst added area-System.Runtime api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jul 27, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 27, 2022
@terrajobst terrajobst added blocking Marks issues that we want to fast track in order to unblock other important work and removed untriaged New issue has not been triaged by the area owner labels Jul 27, 2022
@terrajobst terrajobst added this to the 7.0.0 milestone Jul 27, 2022
@ghost
Copy link

ghost commented Jul 27, 2022

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

Issue Details

From @buyaa-n:

Immo we wonder why the ObsoletedInOSPlatformAttribute has In where others just SupportedOSPlatformAttribute and UnsupportedOSPlatformAttribute, our messaging structure is like

This call site is reachable on {platforms list}. 'The.API' is supported on: {platforms list}

Then the Obsoleted attribute as In but the message will have on:

This call site is reachable on {platforms list}. 'The.API' is obsoleted on: {platforms list}

We might want to change the attribute to similar ObsoletedOSPlatformAttribute or there is specific reason of having In? 

Author: terrajobst
Assignees: -
Labels:

area-System.Runtime, api-ready-for-review
Milestone: -

@terrajobst
Copy link
Member Author

  • We decided to rename it to match other attributes
 namespace System.Runtime.Versioning;

-public sealed class ObsoletedInOSPlatformAttribute : OSPlatformAttribute
+public sealed class ObsoletedOSPlatformAttribute : OSPlatformAttribute

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Aug 2, 2022
@buyaa-n buyaa-n mentioned this issue Aug 9, 2022
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 9, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 9, 2022
jonpryor pushed a commit to dotnet/java-interop that referenced this issue Aug 11, 2022
@jpobst
[generator] Refactor logic for applying [Obsolete] attributes (#1024)
440c05e 
Context: dotnet/android#7234

Refactor logic for applying `[Obsolete]` attributes into a single
common method.  This method will later be extended to add support for
[`[ObsoletedOSPlatformAttribute]`][0].

Doing this piece first and separately allows us to verify that the
refactor does not break anything, as the existing logic is tricky.
A future PR will also remove the temporary hacks used to preserve
stylistic compatibility with a `generator` refactor in 6bbb00a.

[0]: dotnet/runtime#72970
@buyaa-n buyaa-n mentioned this issue Aug 18, 2022
Merged
@rolfbjarne rolfbjarne mentioned this issue Sep 2, 2022
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@buyaa-n buyaa-n

Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime blocking Marks issues that we want to fast track in order to unblock other important work
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

Rename ObsoletedInOSPlatform attribute buyaa-n/runtime
2 participants
@terrajobst @buyaa-n