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

Remotes cleanup refactoring, code cleanup & bug fixing #28

Merged
merged 59 commits into from
Aug 28, 2023

Conversation

averissimo
Copy link
Contributor

@averissimo averissimo commented Aug 1, 2023

Overview of changes description

note: I've added some comments in the code below for additional context.

  • desc_remotes_cleanup() refactoring just as we discussed
    • I couldn't remove Remotes while keeping functionality
  • Adds tests
  • Code reorganization
    • Moved solve_* functions to own file
    • Moved some functions around (especially to desc_util.R, for description-based functions)
    • local_description() moved to R/
      • Useful function to create better examples for developers
  • when resolving packages: Replaces remote_github_ref to CRAN when resolving packages if that version (rule) exists on CRAN
    • For example: formatters (>= 0.5.1) isn't on CRAN, so it keeps the github_ref (instead of replacing with non-existent CRAN versions)
  • Fix Bioconductor lack of published date bug (see comment below)
    • parse_ppm_url() fallback to lastest from PPM if snapshot date cannot be resolved
  • Adds utility map_key_character to avoid repetitive vapply()

R/check.R Show resolved Hide resolved
R/get_ref.R Show resolved Hide resolved
R/utils.R Show resolved Hide resolved
@averissimo averissimo changed the title Training cleanup Remotes cleanup refactoring, code cleanup & bug fixing Aug 1, 2023
@averissimo averissimo marked this pull request as ready for review August 1, 2023 14:40
.pre-commit-config.yaml Outdated Show resolved Hide resolved
R/desc_utils.R Outdated Show resolved Hide resolved
R/get_ref.R Outdated Show resolved Hide resolved
R/get_ref.R Outdated Show resolved Hide resolved
R/get_ref.R Outdated Show resolved Hide resolved
DESCRIPTION Show resolved Hide resolved
@pawelru
Copy link
Contributor

pawelru commented Aug 24, 2023

This is awesome. Thank you! I have approved this. Let's merge it to the training branch and then the training to the main. I will let you decide when you want to pull the trigger.

@averissimo averissimo merged commit cfabadf into training Aug 28, 2023
20 checks passed
@averissimo averissimo deleted the training-cleanup branch August 28, 2023 13:48
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