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
Adding deprecated to @Available directive #851
base: main
Are you sure you want to change the base?
Adding deprecated to @Available directive #851
Conversation
a80b723
to
c2d167f
Compare
d688517
to
8fff1c0
Compare
@mustiikhalil Could you please add the Steps to reproduce section to the PR template, including the expected behaviour and a test DocC catalog that uses the new deprecated keyword? |
Yes, i can do that later today. @sofiaromorales regarding the test bundle i thought it would be enough to include it within the current test bundle which can be found in AvailabilityBundle.docc since its just an arguement to the same directive |
552c4bf
to
86da846
Compare
02e741d
to
b5cc26e
Compare
@sofiaromorales sorry for the ping, but would love to know what's next to get this PR moving forward |
@@ -77,6 +74,7 @@ extension Metadata { | |||
case .iOS: return "iOS" | |||
case .watchOS: return "watchOS" | |||
case .tvOS: return "tvOS" | |||
case .any: return "*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure this is how we want to display any
?
@mustiikhalil can you attach an screenshot on how does this looks on the rendered documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how this is meant to work but I don't think we'd want this to display *
here.
It's a bit odd that the wildcard specifies a version at all because that version may not align across the various platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably say that the correct way would be to list the deprecation on platform where the symbol is known to be available.
However, like I said above, it's a bit odd that we allow specifying a deprecation version for all platforms since the different platforms don't need to align their versions and in practice don't do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fixed now
Tests/SwiftDocCTests/Test Bundles/AvailabilityBundle.docc/ComplexAvailableDeprecated.md
Outdated
Show resolved
Hide resolved
Sorry for the wait. I left some initial feedback and in the upcoming days I'll do another round of reviews. |
@mustiikhalil please update the |
I'll look at it during the weekend. |
dda5175
to
57141c6
Compare
c9d5867
to
4e8cb9d
Compare
Adding deprecated argument to the directive @available and updates documentation to include the new argument
Removes unneeded md file, and updates documentation
4e8cb9d
to
f096f6a
Compare
Summary
Adding
deprecated
argument to the directive@Available
, this is done by adding a fielddeprecated
to theMetadata.Availability
and also addressing all the comments that are related to the issue within the codebase.This PR also include the
.any
platform for@Available
, which can be used as follows:Closes #441
Testing
Adding the following metadata into
SwiftDocC/Directives.md
Using swift-docc-render-artifact
Steps:
Add the above metadata to
Sources/SwiftDocC/SwiftDocC.docc/SwiftDocC/Directives.md
../bin/preview-docs SwiftDocC
Ensure that in the documentation list the "Directives" shows a deprecated tag
Checklist
./bin/test
script and it succeeded(Not sure how to update the docs properly)