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

Add 'min-acl' to let users determine the minimum access level to gene… #28

Merged
merged 4 commits into from Oct 26, 2019

Conversation

minuscorp
Copy link
Contributor

…rate the documentation

Breaking

  • None

Enhancements

  • Adds min-acl parameter that allows private | fileprivate | internal | public | open options, defaults to public as for now.

Bug Fixes

  • None

@minuscorp minuscorp requested a review from eneko as a code owner October 23, 2019 14:20
@minuscorp minuscorp changed the title Add hcl-level to let users determine the minimum access level to gene… Add 'min-acl' to let users determine the minimum access level to gene… Oct 23, 2019
@minuscorp
Copy link
Contributor Author

minuscorp commented Oct 23, 2019

For some reason, sourcedocs generate --min-acl "private" --input-folder Ellu --output-folder Docs -- -workspace Ellu.xcworkspace -scheme Ellu CODE_SIGNING_ALLOWED="NO" does not detect all files in the project, as the command sourcekitten doc -- -workspace Ellu.xcworkspace -scheme Ellu does. Some insight on this? Can someone test in a project/spm module?

@minuscorp
Copy link
Contributor Author

minuscorp commented Oct 23, 2019

So I have tested with a SPM of my own and it works, but I cannot make it work with a Xcode project, sourcekitten seems to return and empty list of source files, while the direct sourcekitten command works, weird

EDIT: It seems that SourceDocs fails to generate the SourceKitten needed steps to work with Xcode projects that use SPM.

@minuscorp
Copy link
Contributor Author

So in the end was a problem with SourceKitten version, updated it, working like a charm. Ready to review

@minuscorp
Copy link
Contributor Author

Not sure why certain steps are failing on the CI (are passing fine on local)

Copy link
Collaborator

@eneko eneko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you for your contribution, @minuscorp.

I had a much basic PR for adding support for open ACL, but was failing too and haven't had a chance to look back into it. I'll run some tests on my end and see if we can figure out how to pass CI.

Thank you


static func == (lhs: AccessLevel, rhs: AccessLevel) -> Bool {
return lhs.priority == rhs.priority
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty cool, nice idea!

@minuscorp
Copy link
Contributor Author

As far as I've tested, this works fine on SPM modules and Xcode projects (the failure was because the outdated Sourcekitten dependency, which ironically makes the build to fail on the CI but not on local 🤷‍♂ )

@minuscorp
Copy link
Contributor Author

minuscorp commented Oct 25, 2019

I think that the issue with the CI can be the usage of a pretty old Xcode image, I world try to bump it at least to Xcode 10, that has support for Swift 5. I think that SourceKitten recently upgraded its minimum Swift version. Giving it a shot doesn't hurt anybody, will try tomorrow.

@minuscorp
Copy link
Contributor Author

Updated the Travis workflow, minimum toolchain needed is Swift 5.0 (SourceKitten)

Copy link
Collaborator

@eneko eneko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, a couple of changes for better code standards.

@eneko eneko merged commit 962bf2a into SourceDocs:master Oct 26, 2019
@minuscorp
Copy link
Contributor Author

minuscorp commented Oct 28, 2019

Have a spare week, want me to dive to finish this and make a release? @eneko

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants