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

Prerelease docs site #306

Merged
merged 4 commits into from
Jul 29, 2020
Merged

Prerelease docs site #306

merged 4 commits into from
Jul 29, 2020

Conversation

jtcohen6
Copy link
Collaborator

@jtcohen6 jtcohen6 commented Jul 28, 2020

I added Netlify config (netlify.toml) for branch-based deployment of the long-lived next branch, which will serve prerelease docs.

Here's the site: https://next--docs-getdbt-com.netlify.app/
For prettiness' sake, we'll want to CNAME this to docs-next.getdbt.com, but that's not actually a blocker.

I defined two new environment variables:

  • ALGOLIA_INDEX_NAME: dbt normally, dbt-next for prerelease docs
  • DISCLAIMER: I just added the word "Prerelease " in some prominent places. We could/should add a bright warning banner to the top of the homepage / every page as well.

Also:

  • Added X-robots-tag = "noindex" as a header for the next deployment
  • Netlify already/automatically adds this header for all preview (PR-based) deployments
  • In Netlify, I turned off all branch-based deployments that aren't master or next. So the only deployments we'll have are master (production), next (prerelease), and previews of PRs that target either of those two branches.

My thinking is that we can merge this commit into master with absolutely zero effect, so that the two branches can stay even. Then, I'll merge #302 into the next branch.

@jtcohen6 jtcohen6 requested review from drewbanin and clrcrl July 28, 2020 19:29
@clrcrl
Copy link
Contributor

clrcrl commented Jul 28, 2020

Could we consider adding pre-release as a yaml config to the page headers, and then rendering a warning based on that?

@jtcohen6
Copy link
Collaborator Author

Could we consider adding pre-release as a yaml config to the page headers, and then rendering a warning based on that?

@clrcrl I decided to add a warning banner in lieu of interpolating the word "Prerelease" in arbitrary places. Let me know if you had something quite different in mind!

@clrcrl
Copy link
Contributor

clrcrl commented Jul 29, 2020

@clrcrl I decided to add a warning banner

Very nice! We'll have to remember to remove this when we merge into master :)

Also I noticed that this PR is against master. We probably want to re-jig that. We should name this branch add-pre-release-docs and have it against the next branch instead.

I think we should also have a RELEASE.md file / a note in the pr template about which branch to make your PR against. I'm happy to do those as the current "maintainer of the docs" .

Let me fuss around with it and see how far I get with things.

@jtcohen6
Copy link
Collaborator Author

jtcohen6 commented Jul 29, 2020

Very nice! We'll have to remember to remove this when we merge into master :)

The banner dynamically populates, using Netlify context variables, based on whether the branch being deployed is named next. So merging this code into master wouldn't cause the banner to appear in the main docs site. I made a one-time deployment to test this, from a branch that isn't named next (instead named test-next): https://test-next--docs-getdbt-com.netlify.app/

Also I noticed that this PR is against master. We probably want to re-jig that. We should name this branch add-pre-release-docs and have it against the next branch instead.

My goal in merging this into master now is to prove that the env-aware stuff works and have the branches at the same level before I merge any substantive content to the next branch. I want to avoid accidentally merging anything that doesn't work at release time, when we'll merge commits from next --> master.

I think we should also have a RELEASE.md file / a note in the pr template about which branch to make your PR against. I'm happy to do those as the current "maintainer of the docs" .

Great idea! I think most community-contributed docs should still be against master, since any fixes or clarifications of existing features belong there. We should lock down the next branch, to some extent.

Copy link
Contributor

@clrcrl clrcrl left a comment

Choose a reason for hiding this comment

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

I just added some notes on how to release this.

Once this is merged in we should:

  1. Change the default branch to current, and put branch protection on it. Also edit existing PRs to be against the current branch
  2. Put branch protection on the next branch too. I'm not sure how that will work when we need to rebase next from current, but ¯_(ツ)_/¯

@jtcohen6 jtcohen6 merged commit 684593d into master Jul 29, 2020
nghi-ly pushed a commit that referenced this pull request Feb 13, 2024
REPO SYNC - Public to Private
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