-
Notifications
You must be signed in to change notification settings - Fork 474
Add archived documentation page with v19.2 support #20229
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
Files changed:
|
@ebembi-crdb can you pls link a Jira issue in the description of this PR? it would be good to see the requirements for this task while reviewing and giving feedback |
when i click on the link to the ZIP file in https://deploy-preview-20229--cockroachdb-docs.netlify.app/docs/releases/archived-documentation.html it shows access denied to the cloud storage with the file ![]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looking good, had some specific feedback we should address to bring in line with our other docs
src/current/.bundle/config
Outdated
BUNDLE_BUILD__CBOR: "--with-cflags=-Wno-incompatible-function-pointer-types" | ||
BUNDLE_BUILD__POSIX___SPAWN: "--with-cflags=-Wno-incompatible-function-pointer-types" | ||
BUNDLE_WITHOUT: "production" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does changing the order of this key in this file matter?
src/current/Gemfile
Outdated
@@ -13,6 +13,11 @@ gem "redcarpet", "~> 3.6" | |||
gem "rss" | |||
gem "webrick" | |||
gem "jekyll-minifier" | |||
gem 'csv' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these being added to silence some warnings?
i would prefer this and the changes in the bundler config be in a different PR that targets resolving those issues
@@ -1064,6 +1064,16 @@ div#feature-highlights .feature-description { | |||
} | |||
} | |||
|
|||
|
|||
.sidebar__tag--new { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think this PR is the place for us to add a "new" tag, we need to discuss with the team since it's a style decision. if we want to add a new sidebar callout tag, we should do that in a different PR
although it is important for us to make the archived documentation available, i don't think we should be highlighting or calling it out as something users need to pay special attention to vs. other things in the docs
}, | ||
{ | ||
"title": "Archived Documentation", | ||
"tag": "NEW", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please see other comment re: adding this "new" tag
@@ -136,8 +128,19 @@ | |||
.attr("href", urls[0] || "#") | |||
.html(item.title); | |||
|
|||
// ── **INJECT TAG BADGE HERE** ───────────────────────── |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see my other comment re: the "new" tag
<tr><th>Version</th><th>Release Date</th><th>EOL Date</th><th>Status</th><th>Downloads</th></tr> | ||
</thead> | ||
<tbody> | ||
<tr><td><strong>v19.2</strong></td><td>November 2019</td><td>May 2021</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove the columns with 'Release Date' and 'EOL Date' since that information is officially maintained on the Release support policy page which is generated from releases.yml
, and we don't want to manually maintain a duplicate here
however, in my other comments the updated text has links to that page if the user wants to review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think we need the 'status' column either, this would only make sense if we were going to have other statuses than "Archived" on this page which as far as i can tell we are not
--- | ||
|
||
<!-- ✨ page-level style overrides --> | ||
<style> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in another comment, i suggest removing the dropdown since it doesn't seem to work, and repeats what is already in the table of ZIP files
then i think a lot of this inline style can go away if we remove the dropdown
title: Archived Documentation | ||
summary: Documentation for CockroachDB versions that are no longer actively maintained | ||
|
||
toc: false # hide “On this page” nav |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment in the YAML header is unnecessary
|
||
</div> | ||
|
||
## Looking for Current Documentation? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest changing to sentence case per style guide, i.e.,
## Looking for current documentation?
...
<tbody> | ||
<tr><td><strong>v19.2</strong></td><td>November 2019</td><td>May 2021</td> | ||
<td><span class="version-pill version-archived">Archived</span></td> | ||
<td><a href="https://storage.googleapis.com/crdbdocs-archive/archive_19.2.zip" class="download-link">ZIP</a></td></tr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this link doesn't work due to object storage permissions denied (see screenshot in another comment)
/releases/archived-documentation.html
ZIP download link
page
interface
Jira link - https://cockroachlabs.atlassian.net/browse/EDUENG-54