Add v1.1 migration guide #1272
Add v1.1 migration guide #1272
Conversation
✅ Deploy Preview for docs-getdbt-com ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
This is awesome @jtcohen6, nice work! Added one requested change for the useEffect in the DocPage file. Once that is adjusted i'd call this good to go.
website/src/theme/DocPage/index.js
Outdated
| useEffect(() => { | ||
| // If version is not isPrerelease, do not show banner | ||
| if(!isPrerelease) { | ||
| setPreData({ | ||
| showisPrereleaseBanner: false, | ||
| isPrereleaseBannerText: '' | ||
| }) | ||
| } else { | ||
| setPreData({ | ||
| showisPrereleaseBanner: true, | ||
| isPrereleaseBannerText: `This is a prerelease version. The latest stable version is ${latestStableRelease}` | ||
| }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
For this useEffect, it looks like this is causing a Maximum update depth exceeded error, due to not having a dependency array.
There's two options to resolve this from what I see. You can either:
- Add the dependency array to the end of this useEffect, making line 75
}, [dbtVersion]) - Or, move the if/else statement from this useEffect, into the useEffect below which uses the same
[dbtVersion]dependency array
This should prevent the useEffect from getting stuck in a constant loop, and only firing when the version has changed.
There was a problem hiding this comment.
@JKarlavige Ah, so that's what }, [dbtVersion]) is doing! There's a reason you're the one who gets to do this stuff for real ;)
Thanks for the quick pointer. I like option 2, since that keeps the logic a bit more consolidated. We shouldn't ever have a version that's both a prerelease and within 3 months of EOL.
website/src/theme/DocPage/index.js
Outdated
| } else { | ||
| setPreData({ | ||
| showisPrereleaseBanner: true, | ||
| isPrereleaseBannerText : `This is a prerelease version. The latest stable version is ${latestStableRelease}` |
There was a problem hiding this comment.
@JKarlavige or @jtcohen6 How can we programmatically add the prerelease version here?
| isPrereleaseBannerText : `This is a prerelease version. The latest stable version is ${latestStableRelease}` | |
| isPrereleaseBannerText : `You are currently viewing v1.1, which is a prerelease of dbt Core. The latest stable version is ${latestStableRelease}` |
There was a problem hiding this comment.
@runleonarun I think I can do this! comma ça ?
|
Thanks for the feedback both! I'm going to merge this in, in order to
|
|
Caught one quick thing I needed to fix after merging: ebd0003 |
|
Looks great, nice work 👍 |
resolves #1260
Let's merge this PR, before all other v1.1 updates, since those PRs should update the migration guide.
Description & motivation
In separate commits, should we want to split any of these off into their own PRs:
versionedPagesto show v1.1 migration guide only when v1.1 selected in dropdown (and warn otherwise). This doesn't seem to work as I'd expect, but I'm also not sure it even makes sense for a page like this—we probably do want to show all version guides to all users, no matter which version they're using right now.Major.Minornaming format (with sidebar update + redirect)Pre-release docs
Is this change related to an unreleased version of dbt?
next<Changelog>[New/Changed] in v0.x.0</Changelog>currentYes! Let's change the way we do this: #1261
Checklist
If you added new pages (delete if not applicable):
website/sidebars.jsIf you removed existing pages (delete if not applicable):
website/sidebars.js_redirects