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

[DOC-10097] 24.1 features that require upgrade finalization #18532

Merged
merged 3 commits into from
May 10, 2024
Merged

Conversation

mdlinville
Copy link
Contributor

@mdlinville mdlinville commented May 9, 2024

@mdlinville mdlinville requested a review from rmloveland May 9, 2024 20:07
Copy link

github-actions bot commented May 9, 2024

Copy link

netlify bot commented May 9, 2024

Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit cabf98f
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-api-docs/deploys/663ea13edb2d6300083310ed

Copy link

netlify bot commented May 9, 2024

Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit cabf98f
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-interactivetutorials-docs/deploys/663ea13e470c470008b89cf1

Copy link

netlify bot commented May 9, 2024

Netlify Preview

Name Link
🔨 Latest commit cabf98f
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/663ea13eecd2d90008597d6b
😎 Deploy Preview https://deploy-preview-18532--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@rmloveland rmloveland left a comment

Choose a reason for hiding this comment

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

LGTM with non-blocking suggestions

Suggest getting technical review on this as well. I assume maybe you've had convos elsewhere with engineering team folks re: these are the features that require upgrade finalization? I realize you shared a link to the code but like, idk how to map from the code in that file to what should be in this doc so I'm assuming you do and/or have talked to people who do.

I also looked at old docs PRs for "upgrade finalization" and discovered that a lot of those PRs didn't have eng reviewers so maybe I'm just wrong and it's not necessary? Defer to your expertise in this area ofc but that's just what came to my mind

src/current/cockroachcloud/upgrade-to-v24.1.md Outdated Show resolved Hide resolved
src/current/v24.1/upgrade-cockroach-version.md Outdated Show resolved Hide resolved
@mdlinville
Copy link
Contributor Author

LGTM with non-blocking suggestions

Suggest getting technical review on this as well. I assume maybe you've had convos elsewhere with engineering team folks re: these are the features that require upgrade finalization? I realize you shared a link to the code but like, idk how to map from the code in that file to what should be in this doc so I'm assuming you do and/or have talked to people who do.

I also looked at old docs PRs for "upgrade finalization" and discovered that a lot of those PRs didn't have eng reviewers so maybe I'm just wrong and it's not necessary? Defer to your expertise in this area ofc but that's just what came to my mind

Thanks. In brief, each line in that section of that file was added by an engineer because their feature requires a migration (and thus, finalization). So we go to the correct area of the file and then use Github's handy Git Blame view to see which commit added each line. From the commit I can get to the PR, where I can read about it and find the engineer who authored it or reviewed it and ask them some questions. That's why I was asking you about protected timestamps earlier. :)

@mdlinville mdlinville force-pushed the DOC-10097 branch 2 times, most recently from 1fb48e4 to cabf98f Compare May 10, 2024 22:35
@mdlinville mdlinville enabled auto-merge (squash) May 10, 2024 22:36
@mdlinville mdlinville merged commit 5e36fb3 into main May 10, 2024
7 checks passed
@mdlinville mdlinville deleted the DOC-10097 branch May 10, 2024 22:40
@rmloveland
Copy link
Contributor

rmloveland commented May 13, 2024 via email

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