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 GitHub workflow to publish docs on GitHub #3302

Closed
wants to merge 13 commits into from

Conversation

thejohnfreeman
Copy link
Collaborator

This will update https://ripple.github.io/rippled automatically any time we push to the develop branch. We can always push to the gh-pages branch manually to override it.

You can see an example of the current output at https://thejohnfreeman.github.io/rippled.

I experimented with Read The Docs before this. RTD supports browsing multiple versions of documentation via its Sphinx theme, but the HTML output of Doxygen is completely unaware of this theme. Even if we were ok with supporting just a single version, getting Sphinx to use the raw HTML output of Doxygen requires a custom builder, but RTD only lets us choose one of the three built-in builders (html, dirhtml, and singlehtml).

endif ()

# https://en.cppreference.com/w/Cppreference:Archives
# https://stackoverflow.com/questions/60822559/how-to-move-a-file-download-from-configure-step-to-build-step
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice comment!

../src/ripple/json/README.md \
../src/ripple/json/TODO.md \
../src/ripple/resource/README.md \
../src/ripple/rpc/README.md \
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these READMEs still included in the documentation? I can't seem to find them at https://thejohnfreeman.github.io/rippled/. For instance, where do I find src/ripple/rpc/README.md?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They're all under Related Pages: https://thejohnfreeman.github.io/rippled/pages.html

To find a given document, you need to know its title. I'd like to go through and re-title them all to make them easier to find in the Related Pages list. src/ripple/rpc/README.md is at https://thejohnfreeman.github.io/rippled/md_ripple_rpc_README.html

Copy link
Contributor

@cjcobb23 cjcobb23 left a comment

Choose a reason for hiding this comment

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

@thejohnfreeman Left one comment worth a response. The github action looks good. To be honest, I am not all that familiar with a lot of the things that are going on in this pull request: doxygen itself, cmake and doxygen integration, and docker. So a "looks good to me" from me shouldn't carry too much weight.

@codecov-io
Copy link

codecov-io commented Mar 24, 2020

Codecov Report

Merging #3302 into develop will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3302      +/-   ##
===========================================
- Coverage    70.61%   70.60%   -0.02%     
===========================================
  Files          676      676              
  Lines        54339    54339              
===========================================
- Hits         38374    38368       -6     
- Misses       15965    15971       +6     
Impacted Files Coverage Δ
src/ripple/app/ledger/impl/InboundLedgers.cpp 27.62% <ø> (ø)
src/ripple/server/impl/BaseHTTPPeer.h 87.57% <0.00%> (-3.56%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b54d672...2aa217c. Read the comment docs.

Copy link
Collaborator

@ximinez ximinez left a comment

Choose a reason for hiding this comment

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

Same as @cjcobb23 , this looks good, but I'm outside my area of expertise. The documentation generation on my end works and looks like your example linked above.

docs/Doxyfile Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
ximinez
ximinez previously approved these changes Mar 25, 2020
Copy link
Contributor

@carlhua carlhua left a comment

Choose a reason for hiding this comment

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

Just added one comment regarding to the token. One other comment I have:

I see that we will have new workflows in the future. I recommend we think about how we organize these actions. Is main.yml the best name for this workflow?

.github/workflows/main.yml Outdated Show resolved Hide resolved
@carlhua carlhua requested review from ximinez and cjcobb23 April 3, 2020 13:50

jobs:
job:
runs-on: ubuntu-18.04
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not ubuntu-latest?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just to pin the version so we can reliably build. We can control when we update this way.

@carlhua carlhua added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Apr 6, 2020
Copy link
Collaborator

@ximinez ximinez left a comment

Choose a reason for hiding this comment

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

Updated changes look good.

This was referenced Apr 7, 2020
@manojsdoshi manojsdoshi mentioned this pull request Apr 8, 2020
@thejohnfreeman thejohnfreeman deleted the docs branch April 8, 2020 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants