Skip to content

Direct users to official Linux install instruction #19625

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

Merged
merged 1 commit into from
Jun 13, 2025

Conversation

rmloveland
Copy link
Contributor

... not bespoke curl commands that can break.

Fixes DOC-13739

Copy link

netlify bot commented May 21, 2025

Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit 79cf289
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-api-docs/deploys/684c5a3f44ab6b0008193d12

Copy link

netlify bot commented May 21, 2025

Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit 79cf289
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-interactivetutorials-docs/deploys/684c5a3faff7e60008c6e09b

Copy link

github-actions bot commented May 21, 2025

Files changed:

Copy link

netlify bot commented May 21, 2025

Netlify Preview

Name Link
🔨 Latest commit 79cf289
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-docs/deploys/684c5a3fe4eeed000837f928
😎 Deploy Preview https://deploy-preview-19625--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 project configuration.

@rmloveland rmloveland requested a review from taroface May 21, 2025 20:06
@rmloveland
Copy link
Contributor Author

@taroface i took a shot at just pointing users to the official install instructions & getting rid of the bespoke curl commands. PTAL! Once we get something we're happy with, I'll backport it to our other supported versions v23.1+

@rmloveland rmloveland requested a review from jhlodin May 21, 2025 20:08
@rmloveland
Copy link
Contributor Author

@jhlodin adding you as an FYI since this is a 'deployment & ops' change

@taroface
Copy link
Contributor

To be clear, the intent is to link people to the doc that takes them down a more regularly maintained flow to find the binaries they want, right?

If so, it looks like the Install Linux page links out to the CockroachDB Releases page, and you kinda have to dig for a moment to find this. Would it be friendlier to just link directly to that section (substituting the page version)? eg, https://www.cockroachlabs.com/docs/releases?#v25-2

I don't feel strongly about this but wondering what you and @jhlodin think.

@jhlodin
Copy link
Contributor

jhlodin commented May 22, 2025

+1 to what Ryan said, but also the new target page doesn't actually have any instructions for downloading, extracting, and adding the binary to the PATH. In other words, this now sends people off to another page to be told to run commands that were on the original page.

I think the general idea of updating these snippets to point to the Linux install doc is on-point, but the common download/extract/copy code blocks need to move to the install doc.

@rmloveland
Copy link
Contributor Author

To be clear, the intent is to link people to the doc that takes them down a more regularly maintained flow to find the binaries they want, right?

yes!

If so, it looks like the Install Linux page links out to the CockroachDB Releases page, and you kinda have to dig for a moment to find this. Would it be friendlier to just link directly to that section (substituting the page version)? eg, https://www.cockroachlabs.com/docs/releases?#v25-2

I don't feel strongly about this but wondering what you and @jhlodin think.

i ended up going with a variant of what Joe suggested, update the Linux install page with all the instructions and then link everything to that. PTAL and let me know what you think!

@rmloveland
Copy link
Contributor Author

+1 to what Ryan said, but also the new target page doesn't actually have any instructions for downloading, extracting, and adding the binary to the PATH. In other words, this now sends people off to another page to be told to run commands that were on the original page.

I think the general idea of updating these snippets to point to the Linux install doc is on-point, but the common download/extract/copy code blocks need to move to the install doc.

makes sense, i updated the Linux install page to have all the download/extract/install instructions there, and pointed everything else to that page

i also updated the way we reference VERSION and ARCHITECTURE on that page in a few places because even though it's nice we can refer to specific versions using the {{page.version.version}} Liquid tag, it is wrong sometimes (e.g. due to the reason that led to this PR), and we were also (incorrectly IMO) assuming the world is on x86_64 which is increasingly not true. I don't think it's a huge lift for users to know their own version/arch combo

anyway PTAL and let me know what you think!

@rmloveland
Copy link
Contributor Author

To be clear, the intent is to link people to the doc that takes them down a more regularly maintained flow to find the binaries they want, right?

Yes!

If so, it looks like the Install Linux page links out to the CockroachDB Releases page, and you kinda have to dig for a moment to find this. Would it be friendlier to just link directly to that section (substituting the page version)? eg, https://www.cockroachlabs.com/docs/releases?#v25-2

I saw Joe's comment that he was also in favor of this and I experimented with adding the {{ page.version.version }} to the link on the Linux install page, unfortunately that page variable yields v25.2, not v25-2. I considered writing some more Liquid to make it do that but I fear it would be a very brittle solution vs. the current link which is known good

Re: the overall PR, I just updated it so all the various things now funnel to the Linux install page as @jhlodin LGTM'd that approach in this comment

PTAL - If you guys are happy with this, I will then backport it to other supported versions before merge

However if you want additional changes, please let me know! Happy to fix anything needed

Comment on lines -52 to +63
<div class="highlight"><pre><code class="language-shell" data-lang="shell"><span class="nb">cp </span>-i cockroach-{{ page.release_info.version }}.linux-{ARCHITECTURE}/lib/libgeos.so /usr/local/lib/cockroach/</code></pre></div>
<div class="highlight"><pre><code class="language-shell" data-lang="shell"><span class="nb">cp </span>-i cockroach-{VERSION}.linux-{ARCHITECTURE}/lib/libgeos.so /usr/local/lib/cockroach/</code></pre></div>
Copy link
Contributor

@taroface taroface Jun 10, 2025

Choose a reason for hiding this comment

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

Was the liquid var not rendering appropriately here? I think we would at least need an example of how to properly express the version in the path. The current published version also has here:

In the following commands, replace {ARCHITECTURE} with linux-amd64 for Intel, or with linux-arm64 for ARM.

which seems useful to know. This comment applies to all similar blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was the liquid var not rendering appropriately here?

Unfortunately it gives incorrect information for several weeks a year during the cloud-first release periods, which was what prompted the user to report to us that curl commands in those other instructions were 404ing. I updated it to {VERSION} to follow the same existing pattern as {ARCHITECTURE} since we already have that, and due to the use of {ARCHITECTURE} the URLs on this page were already not "curlable" and had to be edited.

I think we would at least need an example of how to properly express the version in the path

You're right! Made a wrong assumption there. I have updated the page in both places where this is used with the line:

In the following commands, replace {VERSION} with the version of CockroachDB you are installing, and replace {ARCHITECTURE} with linux-amd64 for Intel, or with linux-arm64 for ARM.

The change is in 19116f6

Copy link
Contributor

@taroface taroface left a comment

Choose a reason for hiding this comment

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

One comment, but otherwise LGTM.

@rmloveland
Copy link
Contributor Author

Thanks for the reviews @taroface and @jhlodin !

I'm going to squash and then rebase onto main and then port these changes to the other supported versions following the process in https://cockroachlabs.atlassian.net/wiki/x/eQDr3

@rmloveland rmloveland force-pushed the 20250521-DOC-13739-no-custom-curl-downloads branch from 19116f6 to 80fa888 Compare June 12, 2025 20:36
... not bespoke `curl` commands that can break.

NB. These changes are backported to supported versions v24.1+

Fixes DOC-13739
@rmloveland rmloveland force-pushed the 20250521-DOC-13739-no-custom-curl-downloads branch from 80fa888 to 79cf289 Compare June 13, 2025 17:05
@rmloveland rmloveland enabled auto-merge (squash) June 13, 2025 17:05
@rmloveland rmloveland merged commit 83bcdd7 into main Jun 13, 2025
7 checks passed
@rmloveland rmloveland deleted the 20250521-DOC-13739-no-custom-curl-downloads branch June 13, 2025 17:14
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