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

Recommend workstation backup as step in the install guide #4302

Merged
merged 4 commits into from
Apr 3, 2019

Conversation

cji
Copy link
Contributor

@cji cji commented Mar 27, 2019

Status

Ready for review

Description of Changes

Fixes #4265.

Changes proposed in this pull request: recommend workstation backup as step in the install guide

Testing

Check for grammar, placement of instructions in the guide.

Deployment

Any special considerations for deployment? No.

Checklist

If you made changes to documentation:

  • Doc linting (make docs-lint) passed locally

@conorsch
Copy link
Contributor

Thank you for the contribution, @cji ! Flagging that the lint failure in CI appears to be unrelated to your changes, likely caused by #4301. We'll work on that to get these tests passing and proceed with review.

Copy link
Member

@eloquence eloquence left a comment

Choose a reason for hiding this comment

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

Hi @cji, thanks for putting this together! (The root cause of the CI failure has been resolved.)

This wasn't stated in the issue you're resolving (my bad!), but I think we should recommend backing up all Tails drives after the installation: the Admin Workstation, the Journalist Workstation, and the SVS. The backup doc covers all three, as well.

The Admin Workstation setup chapter may also not be the best time to mention an Admin Workstation backup, as there's still a post-install step the admin has to perform, see https://docs.securedrop.org/en/release-0.12.1/configure_admin_workstation_post_install.html

I think that post-install chapter may be the best one to add the backup section to. I think it's okay to lengthen the chapter title a bit beyond the pain threshold to make that work ("Configure the Admin Workstation Post-Install and Create Backups"). What do you think?

@cji
Copy link
Contributor Author

cji commented Apr 1, 2019

Thanks for the clarification @eloquence! That totally makes sense. I'll re-work this PR to address those changes.

@cji
Copy link
Contributor Author

cji commented Apr 1, 2019

looks like some additional unrelated CI failures? At least, I don't think this PR should affect grsec kernel things 😅

@conorsch
Copy link
Contributor

conorsch commented Apr 1, 2019

looks like some additional unrelated CI failures? At least, I don't think this PR should affect grsec kernel things

Aye! Caused by #4308, specifically. FYI, it's possible to skip the full CI run if you prefix your branch name with docs-; admittedly that pragma is rather hard to find in the dev docs: https://docs.securedrop.org/en/latest/development/documentation_guidelines.html#testing-documentation-changes

No need to resubmit, just mentioning it in case it saves you time on a future contribution. 🙂 We'll kick CI to get the updated logic running for you, hopefully getting to green across the board.

@cji
Copy link
Contributor Author

cji commented Apr 1, 2019

@conorsch oh cool! I will keep that in mind for next time. Thank you!

Back Up The Workstations
------------------------------------

USB drives can wear out, get lost, or otherwise become corrupted, making it very important to be sure to keep current backups. Follow the :ref:`Backup the Workstations <backup_workstations>` document to create a backup of your Secure Viewing Station, Admin Workstation, and Journalist Workstations after you've completed the installation and post-installation steps.
Copy link
Member

Choose a reason for hiding this comment

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

@@ -63,3 +63,8 @@ of the authenticated hidden services, restart Tails and make
sure to enable persistence.

.. _Authenticated Tor Hidden Services: https://www.torproject.org/docs/tor-manual.html.en#HiddenServiceAuthorizeClient

Back Up The Workstations
Copy link
Member

Choose a reason for hiding this comment

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

Nit: The -> the (title case per https://docs.securedrop.org/en/release-0.12.1/development/documentation_guidelines.html although we're admittedly not super-consistent about this).

@eloquence
Copy link
Member

LGTM, couple of nits noted inline if you have time, if not, we can fix in follow-up. :)

@cji
Copy link
Contributor Author

cji commented Apr 3, 2019

@eloquence Thanks for the review (and for pointing out the style doc, I embarrassingly missed it 😅). I think I addressed your latest comments. Please let me know if I missed anything!

Copy link
Member

@eloquence eloquence left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @cji! Awaiting final review & merge from a maintainer.

@conorsch
Copy link
Contributor

conorsch commented Apr 3, 2019

Lovely. Thank you for the contribution, and for your patience while we got it in, @cji!

@conorsch conorsch merged commit e11503b into freedomofpress:develop Apr 3, 2019
@cji
Copy link
Contributor Author

cji commented Apr 3, 2019

@conorsch no problem, my pleasure! Thanks for all the feedback!

@cji cji deleted the patch-1 branch April 3, 2019 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants