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.

Sign up for GitHub

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

Jump to bottom

doc: update installation documentation #1853

Merged
merged 6 commits into from
Feb 14, 2022
Merged

doc: update installation documentation #1853

merged 6 commits into from
Feb 14, 2022

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Feb 9, 2022

Fixes #1846

Description of the changes being introduced by the pull request:
Update severely outdated installation documentation.

  • Simplify "Simple Installation" section
  • Update "Release Verification" section to actually verify a tuf release and with a key of an active maintainer
  • Update and simplify section about non-python dependencies (just point to installation instructions for underlying crypto backends, they are up-to-date and have become a lot easier)
  • Add "Development installation" section

This PR also updates the install section in the contributing doc, plus other small fixes. See commits for details!

Please verify and check that the pull request fulfills the following
requirements:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@lukpueh
doc: update installation documentation
5ec4c95 
Update severely outdated installation documentation.

- Simplify "Simple Installation" section
- Update "Release Verification" section to actually verify a tuf
  release and with a key of an active maintainer
- Update and simplify section about non-python dependencies
  (just point to installation instructions for underlying crypto
  backends, they are up-to-date and have become a lot easier)
- Add "Development installation" section

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh
doc: update install section in contributing doc
4343931 
Replace custom installation section in contribution docs with
pointer to updated installation documentation.

Also configure sphinx autosectionlabel for cross-document refs.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh
doc: minor title changes for rtd navbar
0b2b1bb 
Make contributing document header sentence case for consistency
with other docs and shorten menu name in side navbar to stand out
less.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh
doc: fix link to tox docs
c40bb30 
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@coveralls
Copy link

coveralls commented Feb 9, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 1822869441

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 98.572%
Totals Coverage Status
Change from base Build 1818576706: 0.2%
Covered Lines: 1094
Relevant Lines: 1106
💛 - Coveralls

lukpueh
lukpueh commented Feb 10, 2022
View reviewed changes
docs/INSTALLATION.rst Outdated

Verify release signatures
-------------------------
Copy link
Member Author

Choose a reason for hiding this comment

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

I find this section sticking out a bit too much for installation instructions. Should we shorten it, or at least move it to the bottom of the document?

Copy link
Member

Choose a reason for hiding this comment

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

I think I agree. I'd like to promote signature verification but when the process looks like this (even while handwaving the PGP key lookup part)... I think it breaks the flow of the document and hides the other install options below the fold.

Considering we have three different install methods (normal, native crypto, dev)... maybe list those three first and then verification as the last thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

see 0216740

@lukpueh
Copy link
Member Author

lukpueh commented Feb 10, 2022

See https://theupdateframework--1853.org.readthedocs.build/en/1853/INSTALLATION.html for how the docs look on RTD.

lukpueh
lukpueh commented Feb 10, 2022
View reviewed changes
docs/CONTRIBUTING.rst Outdated

Also see :ref:`development installation instructions <installation:Install for development>`.
Copy link
Member Author

Choose a reason for hiding this comment

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

This reference is not resolved when the rendered document is browsed on GitHub, which is likely to happen because it is the community standard location for a GitHub repo's contributing docs.

A quick fix would be to use the absolute RTD link (see #1781 (comment)).

In the future we might want to think about separating docs for RTD and docs for GitHub (see #1849)

Copy link
Member

Choose a reason for hiding this comment

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

yeah, it does look like sharing docs is tricky and results in issues like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

see cd70e47

@lukpueh lukpueh mentioned this pull request Feb 10, 2022
Open
jku
jku approved these changes Feb 10, 2022
View reviewed changes
Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

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

Nice improvement. I agree with you on the verification comment though, won't object to moving that part further down.

docs/INSTALLATION.rst Outdated

Verify release signatures
-------------------------
Copy link
Member

Choose a reason for hiding this comment

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

I think I agree. I'd like to promote signature verification but when the process looks like this (even while handwaving the PGP key lookup part)... I think it breaks the flow of the document and hides the other install options below the fold.

Considering we have three different install methods (normal, native crypto, dev)... maybe list those three first and then verification as the last thing?

@lukpueh
doc: move verify section in install docs
0216740 
Move release signature verification instructions to bottom of
install docs. The doc is short, so the section is still prominent
enough for promoting verification, but does not break the reading
flow as much anymore.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh
doc: cross-doc absolute link hack
cd70e47 
This is an ugly hack to also resolve the link when the document is
rendered in GitHub, where it is likely to be browsed, because it is
the community standard location for a GitHub repo's contributing
docs.

Coordinate with theupdateframework#1849 to better separate RTD docs with GitHub docs
in the future.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh
Copy link
Member Author

lukpueh commented Feb 14, 2022

@jku, want to take another look at my latest changes? Or should we just merge?

@jku jku merged commit 899b762 into theupdateframework:develop Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jku jku jku approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update installation instructions
3 participants
@lukpueh @coveralls @jku