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

Tweaks to docs + wrap long lines + add function index #559

Merged
merged 20 commits into from
Jan 25, 2024

Conversation

olivroy
Copy link
Contributor

@olivroy olivroy commented Aug 20, 2023

Just a few minor edits to the docs of deprecated functions. Adding hyperlinks to the new functions.

Ref: https://roxygen2.r-lib.org/articles/rd-formatting.html?q=link#function-links

Edit: I added a few more tweaks to docs to ensure that long lines are wrapped, links are present.

Edit2: since this PR mostly makes changes to docs, you may want to take a look at the .Rd diffs.

@olivroy olivroy changed the title Add links for deprecated functions Add links in the deprecated functions docs Aug 20, 2023
@codecov
Copy link

codecov bot commented Aug 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2583cf0) 100.00% compared to head (b1ef486) 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #559   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines         1269      1274    +5     
=========================================
+ Hits          1269      1274    +5     
Files Coverage Δ
R/adorn_ns.R 100.00% <ø> (ø)
R/adorn_percentages.R 100.00% <ø> (ø)
R/adorn_rounding.R 100.00% <100.00%> (ø)
R/adorn_title.R 100.00% <100.00%> (ø)
R/adorn_totals.R 100.00% <100.00%> (ø)
R/as_and_untabyl.R 100.00% <ø> (ø)
R/compare_df_cols.R 100.00% <ø> (ø)
R/convert_to_date.R 100.00% <ø> (ø)
R/excel_dates.R 100.00% <ø> (ø)
R/excel_time_to_numeric.R 100.00% <ø> (ø)
... and 6 more

@olivroy olivroy changed the title Add links in the deprecated functions docs Tweaks to docs + wrap long lines Aug 20, 2023
@olivroy olivroy mentioned this pull request Aug 21, 2023
9 tasks
@billdenney
Copy link
Collaborator

@olivroy, I just merged your other PR into main. Can you please reconcile the changes between main and this PR?

@olivroy
Copy link
Contributor Author

olivroy commented Dec 7, 2023

I will fetch locally to redocument

R/janitor.R Show resolved Hide resolved
Copy link
Collaborator

@billdenney billdenney left a comment

Choose a reason for hiding this comment

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

This looks good to me, overall. I just have a few, minor comments.

R/adorn_title.R Outdated Show resolved Hide resolved
R/adorn_title.R Outdated Show resolved Hide resolved
R/adorn_totals.R Outdated Show resolved Hide resolved
R/adorn_totals.R Outdated Show resolved Hide resolved
R/compare_df_cols.R Show resolved Hide resolved
R/top_levels.R Outdated Show resolved Hide resolved
@olivroy
Copy link
Contributor Author

olivroy commented Dec 7, 2023

thanks for the review! I addressed your comments

@billdenney
Copy link
Collaborator

@sfirke, this looks good to me. Are you good with merging it?

@olivroy olivroy changed the title Tweaks to docs + wrap long lines Tweaks to docs + wrap long lines + add function index Jan 18, 2024
# If no NA in 3rd variable, it doesn't appear in split list
expect_equal(length(dplyr::starwars %>%
dplyr::filter(species == "Human") %>%
tabyl(eye_color, skin_color, gender, show_missing_levels = TRUE)), 2)

# The starwars data set changed in dplyr v 1.0.0 so have two blocks of tests:
Copy link
Owner

Choose a reason for hiding this comment

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

Nice job removing this!

Copy link
Owner

@sfirke sfirke left a comment

Choose a reason for hiding this comment

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

LGTM

@sfirke sfirke merged commit ad52765 into sfirke:main Jan 25, 2024
9 checks passed
@sfirke
Copy link
Owner

sfirke commented Jan 25, 2024

This is great, thanks @olivroy and thanks also @billdenney for the review and questions!

@olivroy olivroy deleted the document branch January 25, 2024 21:15
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.

3 participants