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

Remove tarballs & Docker links for v20.2 and below #19036

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rmloveland
Copy link
Contributor

@rmloveland rmloveland commented Oct 22, 2024

Fixes DOC-11092

Summary of changes:

  • Update releases.yml with a new boolean field

  • Update releases index page to stop showing downloads for versions <=
    v20.2

  • Update individual release pages to remove download links for versions
    <= v20.2

How it works:

When a specific release (or an entire major version's releases) should
be made no longer available for download, add the following k-v pair to
the YAML block for that release:

is_not_downloadable: true

There is now logic in the Liquid templates that build the main releases
page that checks for this YAML key being set to true. If it is set to
true, the releases page will no longer display links to:

  • Pre-built binary downloads
  • Docker tags
  • Release tarballs

Instead, the table cells where such artifacts used to appear will now
display the message:

No longer available for download.

@rmloveland rmloveland marked this pull request as draft October 22, 2024 16:14
Copy link

github-actions bot commented Oct 22, 2024

Files changed:

Copy link

netlify bot commented Oct 22, 2024

Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit 61356a1
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-interactivetutorials-docs/deploys/673677859920bd00088089e4

Copy link

netlify bot commented Oct 22, 2024

Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit 61356a1
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-api-docs/deploys/6736778569615100086009df

Copy link

netlify bot commented Oct 22, 2024

Netlify Preview

Name Link
🔨 Latest commit 61356a1
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/673677852e24f600087c8383
😎 Deploy Preview https://deploy-preview-19036--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.

@rmloveland
Copy link
Contributor Author

@mdlinville maybe this is also an offline convo but wanted to show you what i have so far to get your feedback from a tools POV

in fact let me DM you to set up some time to walk through this PR maybe

@rmloveland rmloveland force-pushed the 20241022-DOC-11092-remove-download-links-prior-to-v20.2 branch 2 times, most recently from b71b6bc to f703a09 Compare November 4, 2024 21:34
@rmloveland rmloveland marked this pull request as ready for review November 4, 2024 21:37
@rmloveland
Copy link
Contributor Author

@jlinder and @celiala i believe this PR implements the "remove binaries, tarballs, and Docker links for <= v20.1" as we discussed in our meeting

See the PR summary for details of how this is accomplished by adding a new variable to each block of the releases.yml file

the rest of the edits beyond the Liquid templates and the YAML file are removing links from old release pages

probably a good place to start verifying the expected behavior is the rendered previews linked from the description

@rmloveland
Copy link
Contributor Author

cc @jaiayu FYI just so you know this is in progress

Copy link
Contributor

@mdlinville mdlinville left a comment

Choose a reason for hiding this comment

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

The approach LGTM. Maybe I am missing it but I can't see the logic where you are using the new field from the YAML.

@rmloveland
Copy link
Contributor Author

rmloveland commented Nov 6, 2024

The approach LGTM. Maybe I am missing it but I can't see the logic where you are using the new field from the YAML.

Yayyyy thanks for the review Matt!!!!

the logic is here (and also in several other places but they're all the same basic Liquid code):

https://github.com/cockroachdb/docs/pull/19036/files#diff-36b3d19ca22faa226c8c9d2c83670eaaa78cc25f1d68a53b890ec01f97f87c41R430-R432

it basically adds another case to the existing if-else blocks below the "is this a cloud-only release" check with another check that is basically "is this release no longer downloadable"

Copy link

@jlinder jlinder left a comment

Choose a reason for hiding this comment

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

For the releases that are being suppressed (20.1 and before) this LGTM. But the v20.2 releases also need to have their tarballs and docker links removed as well (somehow we must have missed clearly indicating the 20.2 releases also needed to be included when we discussed this change 😕).

Fixes DOC-11092

Summary of changes:

- Update releases.yml with a new boolean field

- Update releases index page to stop showing downloads for versions <=
  v20.2

- Update individual release pages to remove download links for versions
  <= v20.2

How it works:

When a specific release (or an entire major version's releases) should
be made no longer available for download, add the following k-v pair to
the YAML block for that release:

    is_not_downloadable: true

There is now logic in the Liquid templates that build the main releases
page that checks for this YAML key being set to true.  If it is set to
true, the releases page will no longer display links to:

- Pre-built binary downloads
- Docker tags
- Release tarballs

Instead, the table cells where such artifacts used to appear will now
display the message:

    No longer available for download.
@rmloveland rmloveland force-pushed the 20241022-DOC-11092-remove-download-links-prior-to-v20.2 branch from f703a09 to 950c324 Compare November 14, 2024 22:13
@rmloveland rmloveland changed the title Remove tarballs & Docker links for v20.1 and below Remove tarballs & Docker links for v20.2 and below Nov 14, 2024
Remove v20.2 tarballs and Docker links as well
@rmloveland
Copy link
Contributor Author

@jlinder just updated to remove v20.2 as well, sorry about the confusion on my end, classic off-by-one error i guess

PTAL!

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