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

Remove docs, update CI config & tooling #5520

Merged
merged 3 commits into from
Sep 29, 2020
Merged

Conversation

eloquence
Copy link
Member

@eloquence eloquence commented Sep 22, 2020

Docs removal and tooling changes in support of standalone docs repo. Blocked until https://docs.securedrop.org/ is switched over.

Further towards #5435.

Description

  • Removes docs from repository.
  • Clarifies README.
  • Updates CI config, including removal of docs-specific branch filtering logic (on both docs* and stable branches) and docs version update logic (cf. Add basic version update script securedrop-docs#6).
  • Removes documentation logic form update_version.sh (see Add basic version update script securedrop-docs#6).
  • Updates development requirements to remove Sphinx.
  • Updates i18n_tool.py to require a repo directory when updating the list of languages.
  • Updates update-user-guides target to support a DOCS_REPO_DIR environment variable.

Note to reviewer

  • The commits are quite distinct and you may prefer a commit-by-commit review.

Test plan

  • Observe that ./update_version.sh still operates as expected while no longer attempting to modify documentation.
  • Observe that python3 securedrop/i18n_tool.py update-docs (updates list of languages) is self-documenting and works as expected in combination with a docs repo checkout. Easiest to test by removing a valid language from docs/includes/l10n.txt in the docs repo checkout via a test commit.
  • Observe that make update-user-guides is self-documenting and works in combination with a docs repo checkout.

@eloquence
Copy link
Member Author

Pending CI results, I believe this is ready for formal review now, but keeping in draft mode until the new docs repo is live at https://docs.securedrop.org/

To wrap up the migration, we'll need to also update contributor guidelines and release management docs; Joan and I will tackle that next over in the securedrop-docs repo.

@eloquence eloquence changed the title [WIP] Remove docs, update CI config Remove docs, update CI config & tooling Sep 24, 2020
@emkll emkll self-requested a review September 24, 2020 16:20
@conorsch
Copy link
Contributor

conorsch commented Sep 25, 2020

Took an initial pass through, in conjunction with freedomofpress/securedrop-docs#8

  • 🔴 Observe that ./update_version.sh still operates as expected while no longer attempting to modify documentation.

Testing locally, I see

$ ./update_version.sh 1.8.4
sed: can't read install_files/securedrop-app-code/debian/control: No such file or directory

Which doesn't appear to be a problem with this PR, but one we need to resolve all the same. Blame suggests #5465 broke it, which I merged. Mea culpa! Since 1.6.0~rc1 was already configured in develop, we didn't catch this when branching to release/1.6.0 earlier today, but we'll certainly hit it when bumping to rc2.

  • Observe that python3 securedrop/i18n_tool.py update-docs (updates list of languages) is self-documenting and works as expected in combination with a docs repo checkout. Easiest to test by removing a valid language from docs/includes/l10n.txt in the docs repo checkout via a test commit.
  • Observe that make update-user-guides is self-documenting and works in combination with a docs repo checkout.

For both scripts, it seems reasonable to me to set a default value of "../securedrop-docs" and not complain if that dir is found, but we needn't do that on day one. Most important, running the scripts without any customizations makes it very obvious how to resolve, which is a pleasant experience.

@eloquence
Copy link
Member Author

For both scripts, it seems reasonable to me to set a default value of "../securedrop-docs" and not complain if that dir is found

I was hesitant to that because copying a large number of files or making commits beyond the context of a single repo seemed to me the kind of operation a user should always explicitly parametrize to avoid surprises, but I'm happy to go with whatever the team consensus is on this one.

Remove docs* and stable branch rules

Towards #5435
A new pip-compile run bumps the six dependency, as well.
- Add docs repo support to i18n_tool.py
- Migrate docs edits out of update_version.sh
- Add support for docs repo to update-user-guides target.
  The script is run inside a container that does not have access
  to the repo checkout, so moved the copy logic into the Makefile.
  Renamed script to accurately reflect its revised purpose.
@eloquence
Copy link
Member Author

https://docs.securedrop.org/ is now built from https://github.com/freedomofpress/securedrop-docs , so promoting this to ready. Squashed into three distinct commits.

Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

  • Observe that ./update_version.sh still operates as expected while no longer attempting to modify documentation.
  • Observe that python3 securedrop/i18n_tool.py update-docs (updates list of languages) is self-documenting and works as expected in combination with a docs repo checkout. Easiest to test by removing a valid language from docs/includes/l10n.txt in the docs repo checkout via a test commit.
  • Observe that make update-user-guides is self-documenting and works in combination with a docs repo checkout.

Works great! The scripts are quite helpful when run without the required arguments. During 1.6 release, will watch for opportunities to improve further if any surprises occur.

@conorsch
Copy link
Contributor

Proceeding with merge into develop branch. I don't see any need to port these changes into the release/1.6.0 branch, given that https://docs.securedrop.org/ is already serving from https://github.com/freedomofpress/securedrop-docs.

@conorsch conorsch merged commit eff931f into develop Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants