search

swiftlang / swift-docc-plugin

Swift Package Manager command plugin for Swift-DocC
https://swiftpackageindex.com/apple/swift-docc-plugin/1.3.0/documentation/swiftdoccplugin
Apache License 2.0
316 stars 54 forks source link

Add symbolGraphMinimumAccessLevel option support #82

Closed Kyle-Ye closed 1 month ago

Kyle-Ye commented 3 months ago

Bug/issue #, if applicable:

Close #11

Summary

Add symbolGraphMinimumAccessLevel option support.

The current behavior for it is for library target we assume it to be public and for executable target we assume it to be internal implemented in defaultSymbolGraphOptions.

I propose to add symbol-graph-minimum-access-level option in command line so that we can opt-out the default internal behavior for executable target.

The usage is we want to documentation the executable target via docc bundle but we do not want to expose the internal symbol graph into the documentation.

See https://github.com/apple/swift-mmio/pull/104#issuecomment-2136710721

Dependencies

None

Testing

See ArgumentsSymbolGraphMinimumAccessLevelTests.swift

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

Other Link

Kyle-Ye commented 3 months ago

@swift-ci please test

heckj commented 3 months ago

As the individual who opened #11, I'm hugely in favor of this. I don't know what the intended pattern is in calling a command line option "experimental", but since this has a stable part of the API of DocC for some time, and is actively used and adjusted with "OTHER DOCC FLAGS" in a number of places, I'd very much prefer to drop the "experimental" from the CLI option.

Kyle-Ye commented 3 months ago

I don't know what the intended pattern is in calling a command line option "experimental"

Oh, I think we'll eventually not need experimental too. It's the suggestion I got from @ethan-kusters on swift-docc Slack channel.

Reply from Slack channel: There's not - but https://github.com/apple/swift-docc-plugin/issues/11 is tracking the issue. A PR would be great! Maybe we should start with an --experimental prefixed flag? Ideally I think we'd have configuration in the Package.swift itself.

sofiaromorales commented 3 months ago

Thanks for opening this PR @Kyle-Ye! I'll be reviewing this in the upcoming days :)

rauhul commented 2 months ago

Any updates on this? I'd love to adopt it in swift-mmio

Kyle-Ye commented 2 months ago

Any updates on this? I'd love to adopt it in swift-mmio

I'll take some time later this week to resolve the feedback I receive from the reviewers.

d-ronnqvist commented 2 months ago

Any updates on this? I'd love to adopt it in swift-mmio

I'll take some time later this week to resolve the feedback I receive from the reviewers.

There isn't any blocking feedback. I'd be happy to merge this as is and look at surfacing the new flag in the help text in a later PR.

rauhul commented 1 month ago

Can we rebase and merge this?

Kyle-Ye commented 1 month ago

There isn't any blocking feedback. I'd be happy to merge this as is and look at surfacing the new flag in the help text in a later PR.

Not have time to address all the PR comments. Maybe we can address them later since they are implementation details and there are downstream usage waiting this PR.

Can we rebase and merge this?

Done. I'd like to request @d-ronnqvist for a final approval since I removed experimental prefix.

Kyle-Ye commented 1 month ago

@swift-ci please test

d-ronnqvist commented 1 month ago

I'll take a look later today

Kyle-Ye commented 1 month ago

@swift-ci please test