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

GH-14969: [R][Docs] Enable pkgdown built-in search #36374

Merged
merged 2 commits into from Jul 10, 2023

Conversation

eitsupi
Copy link
Contributor

@eitsupi eitsupi commented Jun 29, 2023

Rationale for this change

Enable local search of pkgdown site.

What changes are included in this PR?

Remove some files that are not currently in use.

Are these changes tested?

There is no test, but the following functions can be run to verify that the site is working properly.

pkgdown::init_site("r")
pkgdown::build_search("r")
pkgdown::build_home("r")

Are there any user-facing changes?

No. Document changes only.

@github-actions
Copy link

⚠️ GitHub issue #14969 has been automatically assigned in GitHub to PR creator.

@eitsupi
Copy link
Contributor Author

eitsupi commented Jun 29, 2023

@github-actions crossbow submit preview-docs

@github-actions
Copy link

Revision: 553603d

Submitted crossbow builds: ursacomputing/crossbow @ actions-3581eef739

Task Status
preview-docs Github Actions

@eitsupi
Copy link
Contributor Author

eitsupi commented Jun 29, 2023

I am wondering if r/pkgdown/templates/navbar.html is also not currently in use and could be removed. (which seems to have nothing to do with fixing this issue)

@kou
Copy link
Member

kou commented Jun 29, 2023

Preview: http://crossbow.voltrondata.com/pr_docs/36374/r/

It seems that search box works. Is this expected behavior?

@eitsupi
Copy link
Contributor Author

eitsupi commented Jun 30, 2023

It seems that search box works. Is this expected behavior?

Yes, I think it is working well.

@kou
Copy link
Member

kou commented Jun 30, 2023

@thisisnic @paleolimbot Can we merge this?

@eitsupi eitsupi changed the title GH-14969: [R][Docs] pkgdown built-in search doesn't work GH-14969: [R][Docs] Enable pkgdown built-in search Jul 4, 2023
@thisisnic
Copy link
Member

thisisnic commented Jul 5, 2023

Sorry for the delay on this, have been on holiday.

I am wondering if r/pkgdown/templates/navbar.html is also not currently in use and could be removed. (which seems to have nothing to do with fixing this issue)

Unsure without looking into this if it's a custom template or not, but open to removing it if it's not used - let's discuss in another issue.

Not a criticism but an FYI - I would have approved this sooner with a bit more information confirming that we no longer use the tabset functionality that is implemented in extra.js, but I had a scan through the code and it appears we no longer use this, so it looks good to remove it.

Thanks for investigating and fixing this @eitsupi, it'll be helpful to have the search functionality working now!

Before I merge this, would you mind removing the unused {.tabset} on line 90 in vignettes/developers/setup.Rmd? If you don't get to it before I get back, I'll push the change to the branch.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Jul 5, 2023
Signed-off-by: SHIMA Tatsuya <ts1s1andn@gmail.com>
Signed-off-by: SHIMA Tatsuya <ts1s1andn@gmail.com>
@eitsupi
Copy link
Contributor Author

eitsupi commented Jul 5, 2023

Thanks for review.

Not a criticism but an FYI - I would have approved this sooner with a bit more information confirming that we no longer use the tabset functionality that is implemented in extra.js, but I had a scan through the code and it appears we no longer use this, so it looks good to remove it.

Sorry, I didn't know extra.js had such a feature.

I believe pkgdown supports tabset by default.
https://pkgdown.r-lib.org/articles/test/rendering.html#tabsets

@eitsupi
Copy link
Contributor Author

eitsupi commented Jul 5, 2023

@github-actions crossbow submit preview-docs

@github-actions
Copy link

github-actions bot commented Jul 5, 2023

Revision: da3372c

Submitted crossbow builds: ursacomputing/crossbow @ actions-61b73bed14

Task Status
preview-docs Github Actions

@eitsupi
Copy link
Contributor Author

eitsupi commented Jul 6, 2023

Preview: http://crossbow.voltrondata.com/pr_docs/36374/r

Where can I look to find out this URL? (I recall it being listed in the GitHub Actions log, but I just looked and couldn't find it)

@kou
Copy link
Member

kou commented Jul 6, 2023

@eitsupi
Copy link
Contributor Author

eitsupi commented Jul 6, 2023

See the summary view: ursacomputing/crossbow/actions/runs/5465384142#summary-14796723867

Oh, I get it. Thanks.

@thisisnic thisisnic merged commit 05620b1 into apache:main Jul 10, 2023
10 checks passed
@eitsupi eitsupi deleted the r-docs-enable-search branch July 10, 2023 09:28
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 05620b1.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[R] pkgdown built-in search doesn't work
3 participants