Skip to content

Pkgdown tweaks #3512

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Conversation

infotroph
Copy link
Member

Description

  • Fix a deployment failure from a directory-not-found caused by a hyphen-vs-underscore typo. Only changing the underscore would have made the checkout step clobber the build directory, so I elected to rename the local build folder -- this is an autogenerated ~cache directory that can be deleted at will; naming it _pkgdown_docs/ makes that clearer than package_documentation/.

Improvements while I was here:

  • Add compiled docs to gitignore and Rbuildignore
  • Organize html by R package name rather than position in the source tree, for easier browsing
  • Add a simple index page

Motivation and Context

Both this and existing PR #3510 involve pkgdown and error messages about a directory not existing, but I think they are not related to each other. @divine7022 do you see any conflicts between these?

A followup step not implemented here: It would be nice to have links from each individual package's front page (e.g. _pkgdown_docs/PEcAn.all/index.html) back to the index of all packages (_pkgdown_docs/index.html). I believe there's a way to do this using pkgdown.yml to add custom links to a package's index sidebar, but have not looked up the details.

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@divine7022
Copy link
Contributor

The PR #3510 already addresses the "directory not found" error in second commit.
It was not a typo error caused by a hyphen vs underscore but a path misidentification. During the Publish to GitHub stage, after checking out the target repo(package-documentation), the script was incorrectly looking for package_documentation directory within the target repo instead of returning to the main PEcAn repo. The fix was updating the path in the rsync command from:

rsync -a --delete pkgdocs/ ${VERSION}/

to:

rsync -a --delete ../package_documentation/pkgdocs/ ${VERSION}/

This ensures the correct source directory is used during deployment.

Additionally, regarding the build_pkgdown.R script, I attempted to leave comments directly in this PR, but in some sections weren’t commentable. So, I included the relevant changes and explanations as a commit in PR #3510 — please take a look and review it.

@infotroph
Copy link
Member Author

The PR #3510 already addresses the "directory not found" error in second commit.

@divine7022 You added the second commit to #3510 after I opened this and tagged you here, so I don't think "already" is the right word there.

If you disgree with my proposal to rename package_documentation to _pkgdown_docs, please say that here instead of pushing changes that conflict with it into a previously-unrelated PR.

@divine7022
Copy link
Contributor

Hello sir @infotroph , sorry if my previous comment was confusing. I was just trying to respond to the PR description above.

I completely agree with your proposed change from package-documentation/pkgdocs to _pkgdown_docs. It's a well-thought-out improvement, because the two-level directory structure like package-documentation/pkgdocs isn't necessary, and using a single-level like _pkgdown_docs makes things much cleaner.

Regarding the use of "already", I understand your point. I was referring to the second commit that was made 4 days ago to address the "directory not found" error during the "Publish to GitHub" stage.

Additionally, regarding the build_pkgdown.R script, I attempted to leave comments directly in this PR, but in some sections weren’t commentable. So, I included the relevant changes and explanations as a commit in PR #3510 — please take a look and review it.

Also, I apologize for pushing a new commit to #3510 after you had opened this PR. It was intended to address some enhancements and suggestions that were difficult to leave as inline comments here. The commit was meant to build on top of your PR, not to conflict with it.

Thanks again for your thoughtful suggestions.

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

Successfully merging this pull request may close these issues.

2 participants