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

Reorganize documentation #2413

Merged
merged 10 commits into from
Apr 26, 2018
Merged

Reorganize documentation #2413

merged 10 commits into from
Apr 26, 2018

Conversation

akx
Copy link
Contributor

@akx akx commented Apr 24, 2018

This PR:

  • Reorganizes all the documentation to live underneath docs/
  • Adds Mkdocs configuration (compatible with Read The Docs!) with Material Design styling courtesy of mkdocs-material.

Why we need it: As a devops person trying to configure ingress-nginx, I grew very frustrated about the seeming (dis)organization of the documentation. There were duplicate files, just random notes smattered into README.md, etc., which felt like a pain.

With this PR merged, someone (likely someone more involved with the project core) could set up Read The Docs to automatically generate a searchable, navigable doc site like this:

nginx-docs

Please let me know if there's anything you'd like changed about the PR, or if you have questions! 🙇

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 24, 2018
@aledbf aledbf self-assigned this Apr 24, 2018
@aledbf
Copy link
Member

aledbf commented Apr 24, 2018

@akx first, thank you for doing this. Documentation is by far the weakest point of ingress-nginx.

Can you sign the CLA?

@akx
Copy link
Contributor Author

akx commented Apr 24, 2018

@aledbf Just signed. :)

@@ -0,0 +1,24 @@
#!/usr/bin/env python3
"""
Copy link
Member

Choose a reason for hiding this comment

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

@aledbf aledbf closed this Apr 24, 2018
@aledbf aledbf reopened this Apr 24, 2018
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Apr 24, 2018
@@ -1,4 +1,4 @@
# Role Based Access Control - RBAC
# Role Based Access Control (RBAC)
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is not linked properly (see live demo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is, it's available in the menus. :)
Where did you find a broken link?

Copy link
Contributor

Choose a reason for hiding this comment

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

I got a 404 when clicking on the corresponding link in the side menu, probably a cached response then!

@aledbf
Copy link
Member

aledbf commented Apr 24, 2018

@akx please run ./hack/verify-boilerplate.sh to get the filename with errors

@antoineco
Copy link
Contributor

antoineco commented Apr 24, 2018

Before this (awesome) PR gets merged, should we consider having that documentation hosted and automatically updated right away? @akx do you know if GitHub pages would be a good candidate?

@aledbf
Copy link
Member

aledbf commented Apr 24, 2018

do you know if GitHub pages would be a good candidate?

I think this is the only option if we want to keep this up to date.

Edit: http://www.mkdocs.org/user-guide/deploying-your-docs/#github-pages

@antoineco
Copy link
Contributor

Yup, you could have a new "Docs" Travis stage ;)

@akx
Copy link
Contributor Author

akx commented Apr 25, 2018

@aledbf Boilerplate fixed. Apparently /usr/bin/env python3 wasn't alright, so I added an assertion to check the script is being run on Py3, not Py2.

Yes, Github Pages should be absolutely fine :) After all, it's just a static site.

@codecov-io
Copy link

codecov-io commented Apr 25, 2018

Codecov Report

Merging #2413 into master will increase coverage by 0.12%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2413      +/-   ##
==========================================
+ Coverage   40.92%   41.05%   +0.12%     
==========================================
  Files          74       74              
  Lines        5248     5252       +4     
==========================================
+ Hits         2148     2156       +8     
+ Misses       2807     2803       -4     
  Partials      293      293
Impacted Files Coverage Δ
internal/file/filesystem.go 40.98% <0%> (-0.4%) ⬇️
internal/ingress/controller/nginx.go 5.09% <0%> (-0.02%) ⬇️
cmd/nginx/main.go 26.2% <0%> (+4.82%) ⬆️

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 959d7b1...8aa9db8. Read the comment docs.

@aledbf
Copy link
Member

aledbf commented Apr 25, 2018

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 25, 2018
@antoineco
Copy link
Contributor

@akx there's a minor conflict to resolve due to the merging of #2409.

@akx
Copy link
Contributor Author

akx commented Apr 26, 2018

@antoineco Rebased.

@antoineco
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 26, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akx, aledbf, antoineco

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit bad526b into kubernetes:master Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants