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

Initial separation of tabyl #588

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

billdenney
Copy link
Collaborator

Fix #240

This PR should import all functions from tabyl before being committed. That will require tabyl being on CRAN.

@billdenney billdenney linked an issue Dec 19, 2024 that may be closed by this pull request
@sfirke
Copy link
Owner

sfirke commented Dec 19, 2024

Nicely done! So first order of business is, get tabyl onto CRAN? They are slower with new package reviews than they are with updates.

We are past due to fix #582 which was closed in #584. I wonder if I should just branch off the repo at 2.2.0 and cherry that one fix in, then submit to CRAN as 2.2.1. That should get a very fast review and maybe beat the holiday rush?

@billdenney
Copy link
Collaborator Author

Yeah, the first order of business is to get tabyl onto CRAN. I agree that this may be better after the janitor update, but if we are lucky, they may review tabyl quickly... Maybe?

I think that we nearly have tabyl to a good state to be a separate package. Maybe try to push it when everything is cleaned up there and see if it (surprisingly) goes quickly?

@billdenney
Copy link
Collaborator Author

@sfirke, If some of the author(s) of janitor were solely focused on tabyl and not other janitor functions, they should probably be removed with this merge (because they will be in the tabyl list). Can you please review that?

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7eaa06d) to head (9594888).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              main      #588    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           27        15    -12     
  Lines         1413       826   -587     
==========================================
- Hits          1413       826   -587     
Files with missing lines Coverage Δ
R/clean_names.R 100.00% <100.00%> (ø)
R/make_clean_names.R 100.00% <100.00%> (ø)

@billdenney
Copy link
Collaborator Author

Before we re-import the tabyl functions and after tabyl is on CRAN, I will test reverse dependencies and notify packages that depend on janitor to switch to tabyl.

I think that this means we will not get this PR into janitor before the required CRAN update. (Let's give ourselves breathing room.)

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.

Breaking off tabyl and adorn_ into their own package?
2 participants