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 from md to html for API docs to be hosted on github pages. #545 #562

Merged
merged 3 commits into from Aug 21, 2018

Conversation

radhikaj
Copy link
Contributor

@radhikaj radhikaj commented Aug 20, 2018

  1. Github pages generates a website for documentation in docs folder. Move documentation from doc to docs/
  2. Remove md API documentation. Replace with HTML API documentation generated by Doxygen. Update CMake for refman and refman-source targets
  3. Remove dox2md tool
    Once documentation is published to github pages, it would look like this: file://minint-0mpgvnu/html/index.html
    Remove API reference .md files from the source tree #545

@radhikaj radhikaj added the bug Issue describes unintended or incorrect behavior in the repo or SDK label Aug 20, 2018
@msftclas
Copy link

msftclas commented Aug 20, 2018

CLA assistant check
All CLA requirements met.

…tml files for API documentation

 #Fix check license and one typo
@anitagov anitagov self-requested a review August 20, 2018 23:00
@radhikaj radhikaj added documentation Issue describes the need for updated or additional documentation for the repo or SDK and removed bug Issue describes unintended or incorrect behavior in the repo or SDK labels Aug 20, 2018
DEPENDS refman
COMMAND dox2md ${CMAKE_CURRENT_BINARY_DIR}/xml/index.xml
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/md
COMMAND cp ${CMAKE_CURRENT_BINARY_DIR}/html
Copy link
Contributor

Choose a reason for hiding this comment

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

Please merge lines 37 and 38 to one line if they are less than 80 characters

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

Copy link
Contributor

@anitagov anitagov 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 -- glad that html files are finally available with the source tree. Minor comment.

Copy link
Contributor

@anitagov anitagov left a comment

Choose a reason for hiding this comment

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

Minor comment -- please try to incorporate. Approved.

Copy link
Contributor

@CodeMonkeyLeet CodeMonkeyLeet left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning all of this up! Primary comments that should be addressed are in docs/refman/CMakeLists.txt.

On a more general level, I just realized this means that we would start checking in a bunch of PNG files over time, and in general, Git history does not deal well with that and it adversely affects repo cloning over time. Have we considered starting to use Git LFS or something similar?

add_custom_target(refman-source
DEPENDS refman
Copy link
Contributor

Choose a reason for hiding this comment

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

This still has a dependency on the external refman project above since you're only doing a copy of the output of that project.

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

DEPENDS refman
COMMAND dox2md ${CMAKE_CURRENT_BINARY_DIR}/xml/index.xml
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/md
COMMAND cp ${CMAKE_CURRENT_BINARY_DIR}/html
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider doing an rm -rf * of the contents of the /html folder before copying. We've had issues in the past where there were just outdated files still sitting in the documentation folder that were for API that no longer existed or were made non-public.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! Thanks. Done.


The xml2md tool is used to generate Markdown format (from Doxygen-generated
XML files). The generated Markdown files are the only ones that are checked
The generated HTML files are the only ones that are checked
into the Github repository. This allows one to browse the documentation from
the Github site.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably useful to clarify as " … browse the documentation from GitHub Pages."

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.

@@ -0,0 +1 @@
347f6a5c0c590e960947b169b226a041
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, what is the md5 hash for and do we need to include it in the copy to the source tree? Also, we don't have a .check-license.ignore entry for *.md5, I wonder why it doesn't get flagged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed md5 files. They are needed to allow for incremental doc generation.

@letmaik
Copy link
Contributor

letmaik commented Aug 21, 2018

Why are these generated files checked into master? This feels very wrong to me. If anything, the generated docs should be hosted in a separate orphan branch gh-pages which will always have a single commit (any updates are force pushed).

@CodeMonkeyLeet
Copy link
Contributor

@letmaik I was curious about gh-pages last night, and there doesn't seem to be strong reason to use them anymore.

Regarding the more specific practice of force pushing (which would require an orphan branch), I don't know if erasing versioning entirely a good idea or standard practice; being able to revert to an earlier iteration always seems like a powerful capability, and an incentive for using GitHub Pages over other solutions.

Remove md5 files

Fix Simon's comments
@letmaik
Copy link
Contributor

letmaik commented Aug 21, 2018

@CodeMonkeyLeet My main concern is that generated files don't belong into the main branch of a repository. The StackOverflow question you reference assumes that Jekyll is used, which means non-generated code is committed, e.g. manually written Markdown files. It is common practice that developers keep such docs directly next to the source code in a docs/ folder, hence GitHub added the option to publish directly from such a folder. But if you have tool-generated files, then using a separate orphan branch (or even repository) still makes a lot of sense. Otherwise, where do you stop? You could commit build outputs like shared libraries, executables, PDF versions of the API docs, but people don't do that as it just blows up the repository.

Your concern about versioning is valid of course, and that is what readthedocs is really good at since it establishes a relationship between git version tags and published documentation snapshots, which are all hosted concurrently and accessible. Unfortunately it doesn't support our scenario as the documentation generation has to happen on readthedocs' servers and for obvious reasons they don't allow arbitrary building of C/C++ code, running tools, etc. They also don't support manually uploading docs yet. Which means that either you ignore versioning and only publish the latest snapshot to GitHub Pages, or you come up with a scheme, probably some folder hierarchy and a master page that links to the different version snapshots. The point for the latter would be to make API docs available for different versions at the same time.

I don't see much point in keeping git history of generated doxygen HTML files (in case you were thinking of hosting just the latest API docs and continuously updating those without force pushing). Note for example that the GitHub Pages deployment feature of Travis CI always does a force push, keeping just a single commit. If you want to indicate when a certain feature/function etc. became available, then this should be part of the documentation itself, e.g. "introduced in version x.y".

My recommendation would be, for now, to maintain a nice set of Markdown doc files (handwritten, introduction, tutorials, FAQ, ...) that are browsable from GitHub, and from there link to the API docs on GitHub Pages. I would use gh-pages as orphan branch and publish a single API doc snapshot which corresponds to the latest stable version. If people want to use older or dev versions, they can still easily generate the docs locally. I would also use a subfolder for GitHub Pages, e.g. /api/ within the URL to indicate that this is not the entry point to the docs, but merely contains the API reference. In the future it would make sense to unify these two types of documentation into one website.

@radhikaj
Copy link
Contributor Author

Merging this PR. We can decide if we want to move to gh-pages in the Monday meeting. As of now we have markdowns for everything except the APIs. We would anyway need to move to HTML for the API documentation. We can either create githubpages off of master/docs or off gh-pages.

@radhikaj radhikaj merged commit 38bfc2c into master Aug 21, 2018
@CodeMonkeyLeet CodeMonkeyLeet added this to Backlog in Public preview via automation Aug 25, 2018
@CodeMonkeyLeet CodeMonkeyLeet added this to the 2018.08 milestone Aug 25, 2018
@CodeMonkeyLeet CodeMonkeyLeet moved this from Backlog to Done in Public preview Aug 25, 2018
@CodeMonkeyLeet CodeMonkeyLeet deleted the radhikajdocs branch September 4, 2018 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issue describes the need for updated or additional documentation for the repo or SDK
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants