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

Code cleanup #136

Merged
merged 7 commits into from
Oct 4, 2024
Merged

Code cleanup #136

merged 7 commits into from
Oct 4, 2024

Conversation

mtmorgan
Copy link
Collaborator

@mtmorgan mtmorgan commented Sep 15, 2024

This PR:

Improves/smooths tests by @mtmorgan

Specific minor uncertainties are indicated as comments below. [edit:resolved]

- export S3 methods, even if corresponding generic is not exported
- document the sentinel "_PACKAGE" rather than using @doctype
- two tests fail because the length of the return value has changed (from
  17 to 18 list elements)

  - hard to know how the return value has changed -- simple additiion? of
    which element? etc
  - assume the change does not break code
  - make more robust by checking for set equality of current and returned
    names

- reduce messages from `readr::read_csv()` by providing
  `col_classes = FALSE` argument
- silence progress in test-search.R by adding `verbose = TRUE`
- 'license' is now a list; print the 'name'
- 'length(x$files)` is incorrect for a data.frame, use `NROW()`
R/print.R Outdated Show resolved Hide resolved
@kuriwaki kuriwaki self-assigned this Sep 18, 2024
@kuriwaki
Copy link
Member

Looks good to me pending the license discussion above.

One R-CMD-check Actions check is erroring out with a system library issue; not sure if it will solve itself in a few days

image

mtmorgan and others added 3 commits September 19, 2024 08:09
@kuriwaki kuriwaki merged commit 9af128a into IQSS:main Oct 4, 2024
6 checks passed
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