-
Notifications
You must be signed in to change notification settings - Fork 65
move cuspatial, cuproj docs under 'inactive projects' #654
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
Conversation
✅ Deploy Preview for docs-rapids-ai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| {%- if api.version-overrides -%} | ||
| **[{{ version_name }} ({{ api.version-overrides[version_name] }})](/api/{{ api.path }}/{{ version_name }})** |
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.
These 2 lines are the only new logic. Basically, "if the entry in _data/docs.yml explicitly specifies versions, just use those directly instead of looking up versions in the data from _data/releases.json".
The rest of the diff here is just expanding this out to multiple lines with indentation, to make it easier to read. This doesn't introduce any visual artifacts like weird spacing or line breaks because I've also introduced a lot more whitespace stripping, via {%- and -%}.
For details on how that works in Liquid, see https://shopify.github.io/liquid/basics/whitespace/
| for VERSION_NAME in $(jq -r 'keys | .[]' <<< "$VERSION_MAP"); do | ||
| for PROJECT in $(yq -r 'keys | .[]' <<< "$PROJECT_MAP"); do | ||
| for PROJECT in $(yq -r 'keys | .[]' <<< "$PROJECT_MAP"); do | ||
| for VERSION_NAME in $(jq -r 'keys | .[]' <<< "$VERSION_MAP"); do |
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.
Switching the order of these so that in the logs, all the updates are grouped by library. I personally think it's more common to look at these logs with a question like "which files were copied for cudf?" than "which files were copied for all of the nightlies?".
Happy to switch this back if reviewers disagree, I don't feel strongly.
bdice
left a comment
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.
One possible typo, otherwise LGTM!
| #### DOCS {% for version_name in versions -%} | ||
| {%- if api.version-overrides -%} | ||
| **[{{ version_name }} ({{ api.version-overrides[version_name] }})](/api/{{ api.path }}/{{ version_name }})** | ||
| {%- elsif api.name == "libucxx" -%} |
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.
Check this? elsif seems unusual.
| {%- elsif api.name == "libucxx" -%} | |
| {%- elif api.name == "libucxx" -%} |
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 was as surprised as you are to learn that in Liquid, it's literally spelled elsif 😅
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.
It's... not Jinja. My bad.
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.
Ha no problem, I only just learned that recently through this work.
|
Checked the build after this PR... this is not working yet. Build link: https://github.com/rapidsai/docs/actions/runs/16807658454/job/47604282737 Noticed this in the logs in the "Post-process docs" section That's not correct. And sure enough, the links at https://docs.rapids.ai/api/ are not doing what we want. They look right:
But for example:
|

Fixes https://github.com/rapidsai/ops/issues/4044 (private issue, sorry)
Contributes to rapidsai/build-planning#197
cuproj,cusignal, andcuspatialdocs links therecuproj/cuspatial, as requested in https://github.com/rapidsai/ops/issues/4044Notes for Reviewers
How I tested this
The preview page looks right to me: https://deploy-preview-654--docs-rapids-ai.netlify.app/api/
I see:
We'll have to merge and deploy this to test the S3 changes, but I did a dry run and the output looks correct.
patch to remove AWS API calls (click me)
With that patch applied, ran:
Saw the expected output... correct versions, nightlies skipped based on configuration in
_data/docs.yml, no duplicate libraries.logs (click me)
Before you review: Liquid is not Jinja!
I learned this doing this PR, maybe other reviewers will already be aware... the templating in this repo looks like Jinja2 (https://jinja.palletsprojects.com/en/stable), but it's actually Liquid (https://shopify.github.io/liquid).
These will be especially useful in understanding this PR's changes: