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 Jazzy Docs #2250

Merged
merged 4 commits into from Aug 22, 2017
Merged

Add Jazzy Docs #2250

merged 4 commits into from Aug 22, 2017

Conversation

ghost
Copy link

@ghost ghost commented Aug 21, 2017

Issue Link πŸ”—

Goals ⚽

  • Add documentation to the repo rather than using (the now deprecated) CocoaDocs

Implementation Details 🚧

  • Adds docs folder with Jazzy generated documentation
  • Adds Jazzy as a developer dependency (via Bundler)
  • Update the readme to point to GitHub Pages (when it is configured in repo's settings) for locally generated Jazzy docs instead of CocoaDocs as it is now deprecated

Testing Details πŸ”

View documentation:

  • Open index.html in the docs folder in a browser

Generate documentation:

  • Run bundle exec jazzy in the root directory of the project

Notes ⚠️

  • To use GitHub Pages for the Jazzy documentation, the repo's settings needs to be updated after merging into master. There's an option in the GitHub Pages configuration to adjust the publishing source to master branch /docs folder. See the GitHub Pages documentation.

Aaron McTavish added 3 commits August 21, 2017 08:37
- Adds Jazzy as a developer dependency using Bundler
- Configures Jazzy for Alamofire

Jazzy repo: https://github.com/realm/jazzy
Update the readme to point to GitHub Pages (when it is configured in repo's settings) for locally generated Jazzy docs instead of CocoaDocs as it is now deprecated.
Copy link
Contributor

@jshier jshier left a comment

Choose a reason for hiding this comment

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

Looks great! I think we could merge this even before Alamofire 5.

Gemfile Outdated
@@ -0,0 +1,3 @@
source "https://rubygems.org"

gem "jazzy"
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're adding a Ruby environment, can you add cocoapods to the Gemfile, and add .ruby-version (2.4.1) and .ruby-gemset (alamofire) files too? That way it will integrate with RVM / rbenv automatically.

author_url: http://alamofire.org/
github_url: https://github.com/Alamofire/Alamofire
module: Alamofire
output: docs
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to refactor our Documentation layout with this. Perhaps we can put these docs in Documentation/jazzy and move the migration guides Documentation/Migration Guides. That will require some Xcode project changes for the guides, but there's no need to add the jazzy docs to Xcode at all.

Copy link
Member

Choose a reason for hiding this comment

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

After more investigation, it looks like it has to go in the master branch in a docs folder to work seamlessly with GitHub Pages. We can sort out where we put everything else in another PR. Might not really be that big of a deal to have a docs directly full of Jazzy docs and then a Documentation folder with everything else nested.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

Gemfile.lock Outdated
jazzy

BUNDLED WITH
1.14.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the latest bundler.

README.md Outdated
@@ -3,7 +3,7 @@
[![Build Status](https://travis-ci.org/Alamofire/Alamofire.svg?branch=master)](https://travis-ci.org/Alamofire/Alamofire)
[![CocoaPods Compatible](https://img.shields.io/cocoapods/v/Alamofire.svg)](https://img.shields.io/cocoapods/v/Alamofire.svg)
[![Carthage Compatible](https://img.shields.io/badge/Carthage-compatible-4BC51D.svg?style=flat)](https://github.com/Carthage/Carthage)
[![Platform](https://img.shields.io/cocoapods/p/Alamofire.svg?style=flat)](http://cocoadocs.org/docsets/Alamofire)
[![Platform](https://img.shields.io/cocoapods/p/Alamofire.svg?style=flat)](https://Alamofire.github.io/Alamofire)
Copy link
Contributor

Choose a reason for hiding this comment

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

It will be a lower case alamofire.github.io.

@jshier jshier requested a review from cnoon August 21, 2017 20:01
@jshier jshier added this to the 4.6.0 milestone Aug 21, 2017
@cnoon
Copy link
Member

cnoon commented Aug 22, 2017

So for my own clarification, would our release workflow change from the last commit being updating the version and finalizing the CHANGELOG to updating the Jazzy docs using bundler? I would assume we'd always want the docs to represent the latest officially released version...but maybe that's not a good assumption.

On a separate note, thanks for putting this together @aamctustwo! 🍻

Copy link
Member

@cnoon cnoon left a comment

Choose a reason for hiding this comment

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

Overall, changes look great @aamctustwo. Once you get these minor comments addressed we can get this merged and cut the 4.6.0 release. πŸ‘πŸ»

.jazzy.yaml Outdated
module: Alamofire
output: docs
theme: fullwidth
xcodebuild_arguments: [-workspace,'Alamofire.xcworkspace',-scheme,'Alamofire iOS']
Copy link
Member

Choose a reason for hiding this comment

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

Spaces after the commas?

README.md Outdated
@@ -45,7 +45,7 @@ Alamofire is an HTTP networking library written in Swift.
- [x] TLS Certificate and Public Key Pinning
- [x] Network Reachability
- [x] Comprehensive Unit and Integration Test Coverage
- [x] [Complete Documentation](http://cocoadocs.org/docsets/Alamofire)
- [x] [Complete Documentation](https://Alamofire.github.io/Alamofire)
Copy link
Member

Choose a reason for hiding this comment

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

Lowercase as @jshier mentioned earlier.

author_url: http://alamofire.org/
github_url: https://github.com/Alamofire/Alamofire
module: Alamofire
output: docs
Copy link
Member

Choose a reason for hiding this comment

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

After more investigation, it looks like it has to go in the master branch in a docs folder to work seamlessly with GitHub Pages. We can sort out where we put everything else in another PR. Might not really be that big of a deal to have a docs directly full of Jazzy docs and then a Documentation folder with everything else nested.

@ghost
Copy link
Author

ghost commented Aug 22, 2017

@cnoon Yes, I'd recommend the final step to be updating the docs.

It might be worth raising a new issue to update the CONTRIBUTING.md file or add a new file in docs/Documentation to write out the release process as it begins to add more steps (as well as looking to possibly script some of the steps).

Responding to PR feedback on #2250

- Adds spacing in array in `.jazzy.yaml` file
- Adds `.ruby-gemset` and `.ruby-version` files for Ruby version managers
- Adds `cocoapods` as an explicit Ruby dev dependency
- Updates Bundler lock to latest version of Bundler
- Updates Readme to use lower-case `ttps://alamofire.github.io`
Copy link
Member

@cnoon cnoon left a comment

Choose a reason for hiding this comment

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

Great work here @aamctustwo...thanks again! 🍻

@cnoon cnoon merged commit 318e590 into Alamofire:master Aug 22, 2017
@cnoon
Copy link
Member

cnoon commented Aug 22, 2017

@ghost ghost mentioned this pull request Aug 23, 2017
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