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 DocC implementation of current docs #59

Merged
merged 5 commits into from Nov 6, 2021

Conversation

Mordil
Copy link
Contributor

@Mordil Mordil commented Oct 31, 2021

Motivation

DocC is a fantastic tool that was announced at WWDC 2021, and full support was released with Swift 5.5

Supporting documentation within DocC gives additional capabilities to developers who work with Time to have better symbol linking and usage guides available while offline, and generated to render within Xcode itself.

Changes Made

  • Add: Documentation catalog to the Time module
  • Add: Documentation articles that copy the text from the existing Documentation/ folder almost verbatim
  • Add: Package manifest variant for Swift 5.5 to support DocC symbol production
  • Change: Base Package manifest to exclude Documentation catalog

Results

DocC support!

@@ -0,0 +1,55 @@
# Common Mistakes

An overview of common mistakes made while dealing with **Time** or its concepts in general.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everywhere in the new markdown, I formatted the package's name to be bold, rather than monospaced to make it stand out more. That's just a personal style of mine, so please let me know if you have a different preference for referencing modules or libraries

These docs are 98% verbatim from their source. I made links to Foundation docs where missing, as well as used DocC symbol linking to provide a richer experience.

The only formatting changes I made were to fit within DocC structure, and where it made absolute sense to break up paragraphs.

Copy link
Owner

Choose a reason for hiding this comment

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

👍 that works for me.

@Mordil
Copy link
Contributor Author

Mordil commented Oct 31, 2021

Here's some screenshots!

Screen Shot 2021-10-30 at 22 39 11
Screen Shot 2021-10-30 at 22 39 47
Screen Shot 2021-10-30 at 22 40 15

@Mordil
Copy link
Contributor Author

Mordil commented Oct 31, 2021

My intention was to just seed the current documentation as DocC for now, leaving the content basically as-is with some early richer linking, and then maybe in a later PR actually editing/re-writing the documentation itself to be more of a first-class citizen as DocC.

I'm not sure how you want to handle the docs at Documentation/.

DocC links (obviously) break when trying to view the markdown in GitHub, so it's a bit substandard to not have a webhosted version of the docs, so I had figured you wanted to keep it as-is until hosting the DocC output

Copy link
Owner

@davedelong davedelong left a comment

Choose a reason for hiding this comment

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

Overall this looks great and I'm happy to approve it.

My only question is on the necessity of the Package@swift-5.5.swift file.

Comment on lines +1 to +19
// swift-tools-version:5.5
// The swift-tools-version declares the minimum version of Swift required to build this package.

import PackageDescription

let package = Package(
name: "Time",
products: [
.library(name: "Time", targets: ["Time"])
],
dependencies: [

],
targets: [
.target(name: "Time", dependencies: []),

.testTarget(name: "TimeTests", dependencies: ["Time"]),
]
)
Copy link
Owner

Choose a reason for hiding this comment

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

Is this Package.swift necessary if the primary one is set to request Swift 5.5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pre 5.5, the compiler emits a warning of unused/unrecognized files and suggests explicitly excluding the DocC file.

To be honest, I’m not 100% sure if that warming is emitted when downstream users are building the library as a dependency, but it definitely emits a warning when building the project itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested this by depending on a special branch that doesn't do this conditional package manifest stuff and I can conform that the compiler warning does get emitted when the library is built by a downstream user.

found 1 file(s) which are unhandled; explicitly declare them as resources or exclude from the target
/Users/.../Library/Developer/Xcode/DerivedData/test-crmhlaksfvpqexbqodizljvczqrs/SourcePackages/checkouts/time/Sources/Time/Documentation.docc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, if your Package.swift doesn't somewhere explicitly say 5.5 is a supported build flavor, the DocC output is not generated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, shoot. I'm seeing my mistake - I didn't properly switch the minimum version in question... I'll fix it!

@@ -0,0 +1,55 @@
# Common Mistakes

An overview of common mistakes made while dealing with **Time** or its concepts in general.
Copy link
Owner

Choose a reason for hiding this comment

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

👍 that works for me.


For example, if `today` is `February 29th, 2020`, then:
- `answer1` will produce `April 1st, 2020` (29 Feb + 1 day = 1 Mar; 1 Mar + 1 month = 1 Apr)
- `answer2` will product `March 30th, 2020` (29 Feb + 1 month = 29 Mar; 29 Mar + 1 day = 30 Mar)
Copy link
Owner

Choose a reason for hiding this comment

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

Typo (originally mine, I think): productproduce

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@davedelong davedelong merged commit 3237cc4 into davedelong:master Nov 6, 2021
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