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

Migrate docs to docs.nextstrain.org #620

Merged
merged 16 commits into from
Nov 12, 2020
Merged

Migrate docs to docs.nextstrain.org #620

merged 16 commits into from
Nov 12, 2020

Conversation

eharkins
Copy link
Contributor

Description of proposed changes

This moves augur's:

Related issue(s)

nextstrain/docs.nextstrain.org#26
nextstrain/docs.nextstrain.org#17

Testing

TODO

@codecov
Copy link

codecov bot commented Oct 15, 2020

Codecov Report

Merging #620 (fa384d1) into master (15d6ca6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #620   +/-   ##
=======================================
  Coverage   28.56%   28.56%           
=======================================
  Files          39       39           
  Lines        5367     5367           
  Branches     1320     1320           
=======================================
  Hits         1533     1533           
  Misses       3778     3778           
  Partials       56       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 15d6ca6...fa384d1. Read the comment docs.

@eharkins
Copy link
Contributor Author

eharkins commented Nov 3, 2020

Upon merging this, we will need to implement some redirects. Here is what my understanding of our current domain context and what it means for creating redirects when we merge this PR. Any corrections are appreciated @huddlej and @tsibley - I am still relatively new to read the docs.

I reference below a redirect-tracking spreadsheet for our docs migration called "where pages have moved to" which is here.

Our main read the docs project is at docs.nextstrain.org, which uses that custom domain (instead of nextstrain.readthedocs.io). Since we made nextstrain-augur.readthedocs.io a subproject of docs.nextstrain.org, it will share the custom domain and thus it automatically redirects from http://nextstrain-augur.readthedocs.io/ to https://docs.nextstrain.org/projects/augur/. However, specific pages within http://nextstrain-augur.readthedocs.io/ are not redirected to the same page within https://docs.nextstrain.org/projects/augur/, but they do still render the same document.

@jameshadfield points all of this out in notes in the augur section of the "where pages have moved to" spreadsheet:

Note 1 about augur -- https://nextstrain-augur.readthedocs.io already redirects to https://docs.nextstrain.org/projects/augur/en/stable/, but pages under that domain don't currently redirect
Note 2 about augur -- these pages are all present at https://docs.nextstrain.org/projects/augur/en/stable/... (these URLs are public but not publicised) however once the migrate branch is merged they may well shift around

I don't think any of this will change when we merge the migrate-docs branch of augur. So I don't think we need to put any redirects in place at all for the "stem replacement"s (as they are referred to in the spreadsheet notes) where document location has not changed, since we won't be creating any 404s as far as I can tell for any of those documents when we merge the augur PR.

What will change is that we have moved some pages from the augur's read the docs project to the main docs.nextstrain.org project. So going to those paths within the augur subproject url will result in a 404. So for each of those we will want to add a "page redirect" or an "exact redirect" (it's not clear to me which after reading https://docs.readthedocs.io/en/stable/user-defined-redirects.html carefully).

For example, instead of getting a 404 at https://docs.nextstrain.org/projects/augur/en/stable/faq/translate_ref.html we would like to instead be sent to https://docs.nextstrain.org/en/latest/augur/docs/faq/translate_ref.html. As far as users who have saved the http://nextstrain-augur.readthedocs.io/ version of that url (which doesn't automatically get sent to the docs.nextstrain.org/projects/augur version of that same url), I think this would be covered by the same such redirect, i.e. we would not have to add redirects for both of:

The reason I think that we would not need both redirects is the "From" url in these type of redirects does not include the domain, so I am hoping that it applies to both the default and the custom domains, i.e. read the docs just considers this as

Finally, I think the answer to @jameshadfield's question (from the augur section of the "where pages have moved to" spreadsheet):

Note 3 about augur - will the old doman continue to work with old versions, or do they also need redirects? In other words, will https://nextstrain-augur.readthedocs.io/en/6.4.1 stay around?

is that it will stay around, and pages like the one you point to will continue to exist. I don't think https://nextstrain-augur.readthedocs.io/ urls will ever completely go away, I think they will just stop getting used as more people start using http://docs.nextstrain.org/projects/augur and http://docs.nextstrain.org/ urls.

@huddlej huddlej self-requested a review November 3, 2020 22:51
Copy link
Contributor

@huddlej huddlej left a comment

Choose a reason for hiding this comment

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

I think I have a better sense of how this migration is happening now, based on this PR. This level of reorganization in RTD is new to me, too, so I don't have much advice to offer. Based on the spreadsheet and the redirect docs linked to above, @eharkins's assessment seems reasonable.

My only big content question was about why the intro to Augur is in the FAQ section here.

For future docs development, it does seem like the :ref: and :doc: roles provided by restructured text will be helpful for preventing broken links in the future. This internal coherence or self-awareness of the docs is a big benefit to moving from Markdown to reST.

As a side note, if we retrigger the CI build it should pass now. It was failing due to a recent change in Snakemake's min Python version that has been fixed in the last day.

@@ -0,0 +1,27 @@
# Augur: Nextstrain's Bioinformatics Toolkit
Copy link
Contributor

Choose a reason for hiding this comment

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

I was surprised to find the "introduction to Augur" in the FAQ section of the docs when I think of this as the content for the front page of the Augur docs.

Does this content need to be here for something about RTD subprojects to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was migrated from https://nextstrain.org/docs/bioinformatics/introduction-to-augur but I don't think it makes a ton of sense in this section either. It could be merged with the front page of the Augur docs since it serves the same purpose as you point out. I'm glad to quickly merge their collective content into the augur docs main index page unless you prefer to do it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the content of the current Augur main index page, so I would consider deleting this intro to augur page unless @jameshadfield feels strongly about keeping any of its content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I just removed it, since most of its content felt like it was already addressed in the existing index page. I did update the warning that linked back to docs.nextstrain.org:
Screen Shot 2020-11-04 at 11 40 49 AM

To provide more context (with a few links) about the difference between the subproject and the main docs.nextstrain.org project. I also made it a note instead of a warning since that seemed more appropriate. I also added a link to the augur github repo which was in the deleted index page, since I didn't see one in the existing index page and it seemed helpful:

Screen Shot 2020-11-04 at 11 40 01 AM

* Have a look at some of the tutorials (listed in the sidebar).
Each one will use a slightly different combination of `augur` commands depending on the pathogen.

* Visit the [augur docs](https://nextstrain-augur.readthedocs.io/en/stable) for more information on the bioinformatics toolkit.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we plan to find/replace all instances of this in the Augur docs for this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This page also came from the nextstrain.org docs (https://nextstrain.org/docs/bioinformatics/what-is-a-build) which explains it linking to the augur docs as if it is not a part of the augur docs. I will make a pass through before we merge this to fix cases like this.


* Learn more about the [data formats Augur uses and produces](../../reference/formats/data-formats).

* See [how Augur commands are used in our Zika build](../../tutorials/zika).
Copy link
Contributor

Choose a reason for hiding this comment

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

After reading more about Intersphinx and :doc: roles for reST, these types of links seem ideal for converting to :doc: links, so we can track links within and between projects more easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. In this case I think we can make that happen if I am merging this into the main index page in these docs since that page is reST.

@eharkins
Copy link
Contributor Author

Upon merging this, we will need to implement some redirects.

My updated general RTD redirects understanding is written up here: nextstrain/docs.nextstrain.org#13 (comment)

I have entered in the redirects we plan to implement for this project's rtd configuration in the augur section of our redirect spreadsheet.

As I mentioned above, the pages marked "Simple stem replacement" will not get redirects; there is no way that I know of to redirect https://nextstrain-augur.readthedocs.io/en/latest/faq/translate_ref.html to https://docs.nextstrain.org/projects/augur/en/latest/faq/translate_ref.html since read the docs considers these the same page. It just doesn't automatically redirect the default domain to the custom domain / subproject one for specific pages, it only does this at the root, sending https://nextstrain-augur.readthedocs.io/ to https://docs.nextstrain.org/projects/augur/en/stable/. This seems ok to me, as I find that when I am navigating the docs having entered at a specific page under https://nextstrain-augur.readthedocs.io/, soon enough I have been redirected to https://docs.nextstrain.org/projects/augur (probably by way of visiting the index page). And importantly the pages being served and the theme are the same no matter which base url you are on, so the only difference in experience is the base url might be different if you use a specific url as your entry point.

Copy link
Contributor Author

@eharkins eharkins left a comment

Choose a reason for hiding this comment

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

Merging nextstrain/docs.nextstrain.org#36 would / will mean some URL updates are needed here. In an effort to reduce complexity, I just added them in comment suggestions which we can add, instead of creating yet another PR into this one.

docs/index.rst Outdated Show resolved Hide resolved
docs/releases/migrating-v5-v6.md Outdated Show resolved Hide resolved
docs/releases/migrating-v5-v6.md Outdated Show resolved Hide resolved
docs/releases/v6.md Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/faq/what-is-a-build.md Outdated Show resolved Hide resolved
eharkins and others added 16 commits November 12, 2020 14:03
this will be source at
guides/contribute/augur
using the submodule for
this repo in docs.nextstrain.org
prepare for merging migrate
into master in docs.nextstrain.org
Part of an effort to standardize the appearance of the sidebar in subprojects. Settings chosen based on docs.nextstrain.org.
this page came from nextstrain.org/docs
and so had broken links and language
that didn't make sense in the augur
docs. This fixes those issues to make
it more useful in this new context
some documents are no longer
in this repo or no longer linked
in a toctree so this updates
links to send you to their new
location in docs.nextstrain.org
this relative link did not work
after fetching this doc into
docs.nextstrain.org because the
folder structure is different from
this repo so this changes it to fit
with that of docs.nextstrain.org.
this means the relative link breaks
in this repo but this document is
not rendered in this repo's docs.
This will need to remain this way
or become an absolute link until
we have a unified toctree.
Update links according to URLs resulting from
the merging of nextstrain/docs.nextstrain.org#36
@jameshadfield jameshadfield marked this pull request as ready for review November 12, 2020 01:20
@jameshadfield
Copy link
Member

Merging this PR as @eharkins has added redirects (in RTD) for URLs which have changed in this PR

@jameshadfield jameshadfield merged commit b2ae15f into master Nov 12, 2020
@jameshadfield jameshadfield deleted the migrate-docs branch November 12, 2020 01:22
@huddlej huddlej added this to the Feature release 10.1.0 milestone Nov 12, 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.

3 participants