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

Fix pkgdown #482

Merged
merged 8 commits into from
Nov 27, 2023
Merged

Fix pkgdown #482

merged 8 commits into from
Nov 27, 2023

Conversation

nikosbosse
Copy link
Contributor

@nikosbosse nikosbosse commented Nov 19, 2023

Currently, building the pkgdown fails due to missing keywords. This PR fixes this.

In addition it also

  • makes light edits to the _pkgdown.yaml file, as discussed in Discussion: Which pkgdown function categories do we want to have?  #481. Mostly, however, this PR tries to fix the failing tests for now. I suggest discussing further what exactly the function categories should be in the issue and revisiting at a later point when we have finalised all functions. Current changes include
    • splitting internal helper functions into input checks and others
    • updating some keywords to reflect the fact that we renamed some functions (e.g. functions aren't called score_* anymore and we have some functions plot.* instead of plot_.)
  • it removes some random linebreaks in the code (and a random empty function)
  • also apparently when you change something from @keywords internal to @keywords something-else it starts complaining about documentation issues that it didn't complain about before. So there are also two small fixes (1 typo + 1 undocumented argument)

Note: I noticed a lot of linting issues. I'd say it's cleaner to address them in their own PR, so I created an issue for it: #485

Copy link

codecov bot commented Nov 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b778f8a) 81.21% compared to head (d641893) 81.21%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #482   +/-   ##
========================================
  Coverage    81.21%   81.21%           
========================================
  Files           20       20           
  Lines         1725     1725           
========================================
  Hits          1401     1401           
  Misses         324      324           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nikosbosse nikosbosse marked this pull request as ready for review November 20, 2023 17:01
@nikosbosse nikosbosse marked this pull request as draft November 20, 2023 17:01
@nikosbosse
Copy link
Contributor Author

nikosbosse commented Nov 20, 2023

@seabbs This is in principle fine to review as a hot-fix of the failing pkgdown. I do think, however, that there is more thinking to be done to re-design the pkgdown yaml and make sure we have everything in the categories we want and with the appropriate keywords.

Option 1 would be to review and merge now to get the CI checks succeeding again.
Option 2 would be that I'll revisit and update later once I've done more thinking in the spirit of what we discussed.

It feels aesthetically pleasing to merge it now to get all CI tests to pass again, but ultimately happy to go with either. Do you have preferences?

Update: I think I'd more strongly prefer to merge this now to keep CI from failing which makes me very sad every time I see it...

@nikosbosse nikosbosse marked this pull request as ready for review November 21, 2023 09:12
@nikosbosse nikosbosse requested a review from seabbs November 21, 2023 09:14
Copy link
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

LGTM. I agree we should do a second issue/PR aimed at finalising the structure.

Copy link
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

LGTM. I agree we should do a second issue/PR aimed at finalising the structure.

@nikosbosse nikosbosse merged commit 55e5744 into develop Nov 27, 2023
10 of 11 checks passed
@nikosbosse nikosbosse deleted the fix-pkgdown branch November 27, 2023 11:49
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.

2 participants