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

Move most stuff to a lib #40

Closed
wants to merge 6 commits into from

Conversation

designatednerd
Copy link
Contributor

Enhancements

  • Created a SourceDocsLib which can be imported separately from the executable, and used within Swift Package Manager Scripts
  • Made a bunch of stuff public to facilitate this

Bug Fixes

  • Reset the markdown index when generation exits to allow multiple generation runs in one go without all targets getting all markdown
  • Enabled the access level tests

Further comments

  • For some weird reason this is only working with my project when I do swift run MyProject and not when I build my project in Xcode. When I build in Xcode i'm getting Please confirm that xcodebuild is building a Swift module.. I think from looking at a bunch of issues on Jazzy, this is probably related to SourceKitten, which they also depend on. However, I think this is at least workable in terms of running on the command line similarly to the previous version.
  • Tests are passing when run via swift test but will fail if run via Xcode - the system lib seems to not really enjoy being run from Xcode.
  • Commandant and a bunch of other stuff being used here just pipe everything out to stdout, which makes it awfully hard to test. I know there's ShellOut that returns strings in addition to sending them to the console, which makes it a LOT easier to test that things are working correctly.

I'm good with this getting merged as a phase one since it works, but wanted to share those notes for use in terms of future rethinks.

@designatednerd
Copy link
Contributor Author

Addresses #39

@designatednerd
Copy link
Contributor Author

Poked Travis settings to use Swift 5.1, the only thing still failing is swiftlint because it's on strict - didn't want to turn that off without checking first, but that does make it fail the build even for warnings. Should I make alterations or turn that off?

@eneko
Copy link
Collaborator

eneko commented Apr 17, 2020

Hi @designatednerd, I'm looking into this (almost two months later, I know 🙈).

First of all, thank you very much for doing this. I think it's a great change for the repo.

Few changes I'd like to do before merging:

  • Looks like a lot of the command handling code has been moved to the library. While the core logic should be moved, command-related code should be for the command line tool only.

I'm going to try to push this changes, but never done that on a fork PR before, so not sure how it works. Will see.

@eneko
Copy link
Collaborator

eneko commented Apr 17, 2020

Looks like I cannot push to this PR, so a new one has been created: #41

@designatednerd
Copy link
Contributor Author

So the problem with moving the command-handling code back to the library is that I can't actually call any of the commands outside the library if we do that. Here's how I'm using it now. If there's a way to separate the options stuff from the commander stuff, that'd probably be best, but I didn't see a straightforward way to do that.

Basically this isn't all that helpful if I can't run the commands from outside the CLI 🙃

@eneko
Copy link
Collaborator

eneko commented Apr 18, 2020

I see, thanks for the link. Yeah, a new public interface will have to be defined. The idea is that the library part would not need to depend on Commandant, Rainbow, or any other command line related code.

I can take care of that, not sure how soon but will work towards it. Will ping you when I have something ready! :)

@eneko
Copy link
Collaborator

eneko commented Apr 27, 2020

Hi @designatednerd,

I've submitted a new pull request to move code into SourceDocsLib. Please check out #42 and let me know your thoughts.

You would need to update your code in apollo-ios like this:

let options = DocumentOptions(moduleName: target.name,
                              linkEndingText: "/",
                              inputFolder: sourceRootURL.path,
                              outputFolder: outputURL.path,
                              clean: true,
                              xcodeArguments: [
                                "-scheme",
                                target.scheme,
                                "-project",
                                "Apollo.xcodeproj"])

do {
  try DocumentationGenerator(options: options).run()
  CodegenLogger.log("Generated docs for \(target.name)")
} catch {
  CodegenLogger.log("Error generating docs for \(target.name): \(error)", logLevel: .error)
  exit(1)
}

eneko added a commit that referenced this pull request Apr 27, 2020
#### Breaking
- Add `SourceDocsLib` so it can be used by other Swift tools to generate documentation.

#### Enhancements
- Migrated from `Commandant` to [`Swift Argument Parser`](https://github.com/apple/swift-argument-parser) for command line parsing

#### Bug Fixes
- None

### Discussion
This pull requests is based in the work done by @designatednerd on #40, to move most code to `SourceDocsLib`, a new library that can be imported  y other Swift applications to generate documentation.

With the migration from **Commandant** to **Swift Argument Parser**, this pull request also sets the base for version 1.0.0. Command line arguments have not changed from prior versions of the tool.
@eneko
Copy link
Collaborator

eneko commented Apr 27, 2020

Closed in favor of #42

@eneko eneko closed this Apr 27, 2020
AF-pfo pushed a commit to AF-pfo/SourceDocs that referenced this pull request Nov 16, 2020
#### Breaking
- Add `SourceDocsLib` so it can be used by other Swift tools to generate documentation.

#### Enhancements
- Migrated from `Commandant` to [`Swift Argument Parser`](https://github.com/apple/swift-argument-parser) for command line parsing

#### Bug Fixes
- None

### Discussion
This pull requests is based in the work done by @designatednerd on SourceDocs#40, to move most code to `SourceDocsLib`, a new library that can be imported  y other Swift applications to generate documentation.

With the migration from **Commandant** to **Swift Argument Parser**, this pull request also sets the base for version 1.0.0. Command line arguments have not changed from prior versions of the tool.
@designatednerd designatednerd deleted the master branch December 24, 2020 03:57
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