Remove new_qenv - #222
Conversation
|
✅ All contributors have signed the CLA |
Code Coverage SummaryDiff against mainResults for commit: c0d6817 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 9 suites 1s ⏱️ Results for commit c0d6817. ♻️ This comment has been updated with latest results. |
|
I have read the CLA Document and I hereby sign the CLA |
|
@llrs-roche After the upcoming release we might not have another release for a very long time. We had last release on CRAN in January 2024. I don't want to wait another year to get this function removed. I think 1 year is enough : ) |
|
@llrs-roche one of the CI checks is failing. Can you check what's going on :) ? and what changes you need to provide to push this through the finish line? |
|
Fixed the issue with the failing test on pkgdown. This check is the one I have as a hook on pre-push on my projects 😄 For finalizing the PR, I should make sure of teal.gallery work well with the new changes (and other code/packages), and probably also check that it meets the Acceptance Criteria. |
|
@llrs-roche yes please check tlg-catalog and teal.gallery so that they do not use |
new_qenv
|
I think |
m7pr
left a comment
There was a problem hiding this comment.
Hey @llrs-roche - last comment in here #222 (comment)
Otherwise this is good to go
Pull Request
As commented on #215, it was hard to see a pattern on what to do when deprecating functions.
As
new_qenv()was important, I decided to keep the code/help page and remove it later on.I considered where to document the function; I think having a separate help page will be easier if other methods are deprecated.
Removing all the arguments from the deprecated function was an idea I had because they won't be used, but this created problems with generic not matching actual method, so I left it as is.
I'm having some issues with pre-commit and it didn't pass the checks, but I think this PR is minimal and won't be any issues
Error messages running pre-commit
Fixes #215