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

Include Phoenix guides with framework API docs #1960

Merged
merged 1 commit into from
Nov 5, 2016

Conversation

wsmoak
Copy link
Contributor

@wsmoak wsmoak commented Oct 10, 2016

  • Declare dependency on phoenix_guides (in docs environment only)
  • Add extras page for each guide with custom title and path
  • Make Overview the main page of the docs

See discussion on phoenixframework/phoenix_guides#391

To preview, check out the branch, run MIX_ENV=docs mix docs, and view doc/index.html.

Or view: http://wsmoak.net/phoenix (published at ref 6cb4797)

@wsmoak
Copy link
Contributor Author

wsmoak commented Oct 10, 2016

This is as much as I can do without touching the phoenix_guides repo.

If the guides still need to be publishable at Readme.io, then I would suggest creating an 'exdoc' branch of phoenix_guides so that people can open PRs against it. (Github lets you choose the "merge ... into" branch when opening a PR.)

Suggested next steps:

  • Add an h1 (# in markdown) to each guide. With that in place you can delete most of the title:s in the mix.exs config, because ExDoc will pick that up.
  • Fix the intra-docs links if the page filename has changed.

Notes:

  • I tried path: "testing/channels" but that caused a no such file or directory error. I don't think ExDoc supports subdirectories yet.
  • There's no support for sub-menus either, but you may be able to approximate it by having, for example, a "Testing" page that simply lists the sub-items, and perhaps changing the titles so that they appear indented. (Prefix with .. or > or something.)

@Gazler
Copy link
Member

Gazler commented Oct 10, 2016

This is awesome! Thanks for picking this up again. Hosting a preview is very helpful too.

I've created a branch from master on the guides https://github.com/phoenixframework/phoenix_guides/tree/ex_doc

@josevalim may have some ideas for what is required (if anything) on the ex_doc side of things to support sub sections.

@josevalim
Copy link
Member

Thank you @wsmoak! ❤️

👍 for using h1 for the title. I would also recommend changing the h3 used as titles to a h2 so we can get proper subsections. Finally, I would also recommend doing mix deps.update ex_doc so we move to the latest exdoc. Latest ExDoc will also allows us to publish it as a epub, which is definitely a win. :D

I will look into supporting subsections and directories.

@josevalim
Copy link
Member

josevalim commented Oct 10, 2016

Update 1: We cannot support directories because it breaks assets such as javascript and css (we assume they are in the same directory which is necessary if we want to work on hexdocs and things of sorts).

Update 2: Note that the option :path for each page has been renamed to :filename on latest ExDoc.

Update 3: PR for groups sent here: elixir-lang/ex_doc#613

@Gazler
Copy link
Member

Gazler commented Oct 10, 2016

I added the groups in this commit Gazler@ebaf135 - feel free to pull it into this PR, we should probably wait for ex_doc to be released before finalizing though.

Preview: https://s3.amazonaws.com/gazler.phoenix.guides/a_introduction.html

@wsmoak
Copy link
Contributor Author

wsmoak commented Oct 10, 2016

re: path has been changed to filename I was looking at this doc on ex_doc master: https://github.com/elixir-lang/ex_doc/blob/master/lib/mix/tasks/docs.ex#L77 (still says path). And apparently there is also a group now. :) Thanks @Gazler I'll pull that change in.

@josevalim
Copy link
Member

@wsmoak great catch, all fixed! :D

@wsmoak
Copy link
Contributor Author

wsmoak commented Oct 10, 2016

FWIW, readme.io also flattens the directory structure when it generates the html. Now that we have 'group' to do the submenus it's about the same.

@Gazler
Copy link
Member

Gazler commented Oct 10, 2016

I haven't looked into why this happens yet, but I have found a couple issues by glossing over the guides and just wanted to make a note of them:

The list items about half way down http://wsmoak.net/phoenix/overview.html are collapsed into a single longer list.

phoenix-li-collapsed

The image at the bottom of http://wsmoak.net/phoenix/adding-pages.html#another-new-page is 404ing

phoenix-frank-greets-us

@josevalim
Copy link
Member

Oh, you will almost certainly need to change the structure when it comes to assets. ExDoc allows you to put anything you want inside the doc/ directory, so you can put assets in doc/assets/... and then refer to them as assets/... although we won't copy those automatically. We should likely add a way to copy this stuff though, because we will need to know where the assets are for stuff like epub.

@milmazz, what do you think would be the best way to support assets in the epub formatter? Should we have a directory where supposedly all assets go? Should we just assume it is doc/assets?

@wsmoak
Copy link
Contributor Author

wsmoak commented Oct 11, 2016

It's looking good! I'll open a PR for the h1's against the ex_doc branch of phoenix_guides (unless someone beats me to it).

Once those start appearing I'll drop as many of the title:s in this PR's mix.exs as possible.

Currently the .md files in phoenix_guides are not under the 'docs' directory. I could go either way -- that repo doesn't really need to follow the project directory structure, but doing so and adding a minimal mix.exs there might make it easier for contributors to preview changes locally.

I assume we aren't going to officially release phoenix_guides on hex, but to just point from here to a branch of the docs for each release, so that docs changes can be made after a Phoenix release, and anyone cloning the source will get the updated docs.

It will be far easier for someone with direct access to the repo to do any file renaming and/or moving of assets. I'll keep an eye on the ex_doc branch of phoenix_guides, and adjust the paths in this PR to match.

BTW If you're going to rename things... the .md files don't really need the capital letter prefix that controlled the ordering for readme.io. (And then we could drop the filename:s from mix.exs here.)

@milmazz
Copy link
Contributor

milmazz commented Oct 11, 2016

what do you think would be the best way to support assets in the epub formatter? Should we have a directory where supposedly all assets go? Should we just assume it is doc/assets?

@josevalim I think that the easiest way to go is having a directory where we put all the assets, right now the EPUB formatter puts the logo under the assets directory. So, we can use that directory, wdyt?

.
├── META-INF
│   ├── com.apple.ibooks.display-options.xml
│   └── container.xml
├── OEBPS
│   ├── ExDoc.CLI.xhtml
│   ├── ...
│   ├── ExDoc.xhtml
│   ├── Mix.Tasks.Docs.xhtml
│   ├── assets
│   │   └── logo.png
│   ├── content.opf
│   ├── dist
│   │   ├── app-75ab88af99.js
│   │   └── epub-1ae56b617e.css
│   ├── nav.xhtml
│   ├── title.xhtml
│   └── toc.ncx
└── mimetype

@Gazler
Copy link
Member

Gazler commented Oct 11, 2016

Currently the .md files in phoenix_guides are not under the 'docs' directory. I could go either way -- that repo doesn't really need to follow the project directory structure, but doing so and adding a minimal mix.exs there might make it easier for contributors to preview changes locally.

It would be nice to be able to build the guides in isolation.

It will be far easier for someone with direct access to the repo to do any file renaming and/or moving of assets. I'll keep an eye on the ex_doc branch of phoenix_guides, and adjust the paths in this PR to match.

I'll take this on - I'll drop the letter prefixes whilst I am at it (phoenixframework/phoenix_guides#586)

One additional thing I think we should do as well is move this directory out into its own repo. https://github.com/phoenixframework/phoenix_guides/tree/master/styling (If we use git filter-branch then we should be able to preserve the history too.)

@josevalim
Copy link
Member

josevalim commented Oct 11, 2016

@milmazz we can either ask for an assets directory, outside of doc/assets, and automatically copy everything over to doc/assets OR we can tell them doc/assets should exist and they should put their assets there upfront. I am more inclined towards the latter, sounds good?

@milmazz
Copy link
Contributor

milmazz commented Oct 11, 2016

... OR we can tell them doc/assets should exist and they should put their assets there upfront. I am more inclined towards the latter, sounds good?

@josevalim Sounds good to me 👍

@josevalim
Copy link
Member

@milmazz beautiful. I will ensure it works on master. We just need to guarantee we put the logo there and that we won't warn/raise/fail if the logo is already there. :)

@josevalim
Copy link
Member

@milmazz so I started implementing this and I realized it is a bad. If we mix generated content (like logo.png) with user content, we will be mixing those artifacts in a way that makes it hard to gitignore them. Also, given that epub will have to copy the assets anyway to a separate directory, I am starting to think it is best to ask the user for an assets directory and we will automatically copy everything over.

@milmazz
Copy link
Contributor

milmazz commented Oct 11, 2016

@josevalim Please, let me know if I got this right, as an example, let's assume the current Phoenix Guides:

$ ls deps/phoenix_guides/images/
hello-from-phoenix.png  hello-world-from-frank.png  welcome-to-phoenix.png

The user just need to include the directory deps/phoenix_guides/images in the mix.exs config file (or via mix docs) and then we'll copy the complete structure (including sub-directories) and put it under doc/images (HTML format) or OEBPS/images (EPUB formatter).

If the user also specifies a logo, the logo.{png,jpg} will remain under "doc/assets/" (HTML format), right?

@josevalim
Copy link
Member

josevalim commented Oct 11, 2016

@milmazz we could make it work like that but I have pushed another implementation to master.

In master, if the user specifies assets: "deps/phoenix_guides/images/", we will copy everything inside the images directory, including subdirectories, to the "doc/assets". So, in your example, they will find "doc/assets/hello-from-phoenix.png". If they want an images directory inside the assets directory, then they should have something like assets: "deps/phoenix_guides/assets" and put the "images" directory inside the assets one.

This feature is already on ExDoc master for both html and epub. :) The idea of copying everything into a single directory is to keep everything contained. I am not sure if all formats would allow us to add entries at the root.

@josevalim
Copy link
Member

josevalim commented Oct 11, 2016

@wsmoak @Gazler please give a try, whenever you have some time, to the assets feature in ExDoc master. The idea is the following, if you specify:

assets: "deps/phoenix_guides/assets"

Everything inside "deps/phoenix_guides/assets" will be copied to "doc/assets", which means you will be able to access those images using "assets/name.extension" in your markdown files. For example, if you have "deps/phoenix_guides/assets/images/hello.png" and you specify assets: "deps/phoenix_guides/assets", you should be able to reference the image as "assets/images/hello.png" in markdown.

@wsmoak wsmoak force-pushed the guides_as_mix_dep branch 2 times, most recently from a9ef0b8 to 15c2f7a Compare October 12, 2016 00:47
@wsmoak
Copy link
Contributor Author

wsmoak commented Oct 12, 2016

I added the assets: config and it works as advertised, copying into doc/assets, so that it contains the logo.png that came from the top level of the framework repo, plus the files from in deps/phoenix_guides/images. 👍

I'll wait to see things get re-arranged in the ex_doc branch of phoenix_guides, it sounds like that will adopt the standard directory structure and get some re-naming done.

@josevalim
Copy link
Member

Thank you @wsmoak. ExDoc v0.14.3 has been released with support for assets and for grouping guides together.

@ybur-yug
Copy link
Contributor

@josevalim @Gazler @wsmoak I'd be happy to help with this if theres anything that I could throw in that would be useful. I love working on the guides and would be very happy to help contribute to this further and make the transition easy :)

Let me know. I also was curious about the roadmap for the 1.3 updates for the guides since the new project structure etc will require a lot of updating of smaller things.

@wsmoak
Copy link
Contributor Author

wsmoak commented Oct 13, 2016

Thanks, I've updated to the ex_doc release.

If someone has time to review and merge phoenixframework/phoenix_guides#594 that adds the h1 headings, then I can remove the title:s from the config here, because they will get picked up automatically and used as the menu items.

@Gazler
Copy link
Member

Gazler commented Oct 14, 2016

@ybur-yug I appreciate the offer. Once this PR has been merged and the Phoenix 1.3 generators are finalized, I am sure there will be quite a few changes required to the guide. Many of the file paths and contents will change for generated files.

This PR and #1927 are the relevant ones.

@ybur-yug
Copy link
Contributor

@Gazler Thats what I thought. I had started this just to get a feel for it. I will keep my eyes on that PR and once ready from there can take a stab at a first draft updating the minutiae of the listings of dirs, names of files generated, structure, etc and such and go from there if that works.

@wsmoak wsmoak force-pushed the guides_as_mix_dep branch 2 times, most recently from a5220ef to 198a0b6 Compare October 22, 2016 17:28
@wsmoak
Copy link
Contributor Author

wsmoak commented Oct 30, 2016

This is waiting on the ex_doc branch of phoenix_guides [1] moving to master.

See discussion: phoenixframework/phoenix_guides#391 (comment)

(I don't know what's up with the test failure on this though... I've only touched the mix files.)

[1] which moves the files down into 'docs' and adds h1 headers to each page

- Declare dependency on phoenix_guides (in docs environment only)
- Add extras page for each guide with custom group
- Titles are taken from new h1 elements on guide pages
- Make Overview the main page of the framework docs
- Set preferred environment for `docs` task so you don't have to type `MIX_ENV=docs`, just `mix docs` will work

See discussion on phoenixframework/phoenix_guides#391
@wsmoak wsmoak changed the title Include Phoenix guides in framework docs Include Phoenix guides with framework API docs Nov 3, 2016
@wsmoak
Copy link
Contributor Author

wsmoak commented Nov 3, 2016

The ex_doc-related changes to phoenix_guides have been merged to master (thanks @Gazler !). I updated this to pull in the default branch of phoenix_guides as a git dependency, so it's grabbing master, currently at 4ec8022 .

Here are the latest generated docs as a preview: http://wsmoak.net/phoenix/overview.html

This PR is ready to merge to master any time.

As discussed in phoenixframework/phoenix_guides#391 (comment) the docs are not yet perfect, some h3's need to be h2's and some links are probably broken, but it will be much easier for people to help if it's all on master of both repos.

Let me know what you think and/or if you want any changes to this!

@chrismccord chrismccord merged commit 2ce2986 into phoenixframework:master Nov 5, 2016
@chrismccord
Copy link
Member

Thanks so much @wsmoak! Sorry for my delay on this discussion, but I'm just about caught up on my backlog and ready to help iron out any docs issues we come across. <3 <3 <3

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.

6 participants