Pre-release activities

[donyunardi]

Package level checks

• 624 PACKAGE LEVEL pre-release activities #639
• Review and update:
  • README.md (check example code)
  • NEWS.md
• Run urlchecker::url_check() to identify broken links and fix
• Run R CMD check --as-cran make sure everything pass
• Package Title is not duplicated in Package Description in DESCRIPTION file (e.g. this happens in teal.slice currently)
• All package names in Title and Description fields of DESCRIPTION file are quoted with '
• You have checked the Package Release Template https://github.com/insightsengineering/teal.reporter/pull/205/files
• Make sure all teal.* mentions are lower-cased and quoted
• Make Sure inst/WORDLIST is minimalized
• Make sure each link to our documentation hosted with pkgdown on github pages do not have /main/ in the address but it has /latest/ instead, so we always expose the documentation of the latest release and not what's currently on main branch but not yet released,
• Remove exception in .lintr: indentation_linter = NULL
• Remove @nomd (in favor of Roxygen: list(markdown = TRUE) in DESCRIPTION)

Vignettes level checks

• Pre-release activities - vignettes review #640
• Sanity check of all vignettes, make sure there is no typo, no wrong format, etc.

Non-exported functions level checks

• Pre-release activities - non-exported functions review #641
• Make sure non-exported functions do not have examples

Exported functions level checks

• Review functions:
  • @example tag, make sure it runs, fix if otherwise
  • Make sure functions has @return tag to document the return value
  • no \dontrun tag, replace with if(interactive()) if needed
  • Standard order of roxygen2 tags
  • @title ➡️ @doctype ➡️ @description ➡️ @details ➡️ @rdname ➡️
  • ➡️ @inheritParams ➡️ @params ➡️ @return ➡️ @seealso ➡️ @references ➡️
  • ➡️ @examples ➡️ @export ➡️ @keywords ➡️ @noRd
• Make sure there are no ::: in examples
  • if you need to retain the example that uses :::, use getFromNamespace() function.
• Remove return wrapper if it is the last expression (per NEST guidelines)

Please self-assign by your name next to the module and link to the PR:

• tm_g_response.R @averissimo pre-release tm_g_response document update #666
• tm_file_viewer.R @averissimo pre-release tm_file_viewer document update #665
• tm_t_crosstable.R @averissimo pre-release tm_t_crosstable document update #664
• tm_g_distribution.R @kartikeyakirar pre-release tm_g_distribution document update #669
• tm_data_table.R @kartikeyakirar pre-release tm_data_table document update #667
• tm_missing_data.R @kartikeyakirar pre-release tm_missing_data document update #668
• tm_g_bivariate.R @kartikeyakirar pre-release tm_g_bivariate document update #660
• tm_g_scatterplotmatrix.R @kartikeyakirar pre-release tm_g_scatterplotmatrix document update #661
• tm_outliers.R @kartikeyakirar pre-release tm_outliers document update #653
• tm_g_scatterplot.R @kartikeyakirar pre-release tm_g_scatterplot document update #657
• tm_front_page.R @kartikeyakirar pre-release tm_front_page document update #654
• tm_a_pca.R @kartikeyakirar pre-release tm_a_pca document update #646
• tm_variable_browser.R @kartikeyakirar pre-release tm_variable_browser document update  #652
• tm_a_regression.R @kartikeyakirar pre-release tm_a_regression document update #655
• tm_g_association.R @kartikeyakirar pre-release tm_g_association document update #656
• Rest of R files @kartikeyakirar pre-release other utils functions document update  #676

extra:

• Add common @return tag Adds common @return for modules and corrects linting #670
• Standard function title format in docs Standardise function titles similar to what we have in tmc #691

Potential removal of dependencies

Consider:

• Pre-release activities - Potential removal of dependencies #643
• Removal of {magrittr} by importing pipe operator from {dplyr} ✔️ done
• Other packages that can be easily removed ✔️ removed tidyselect as its functions are reexported from dplyr
• Move any of the packages from Depends -> Imports
  • {ggmosaic} ❌ impossible (see on issue)
  • {ggplot2} ❌ impossible (see on issue)
  • {shinyTree} ✔️ done
  • {teal.transform} ❌ valid: teal.transform functions should be available to tmg users
  • {teal} ❌ valid: teal dependency is standard in all module packages
  • {shiny} ❌ valid: like teal, module packages use shiny extensively
• Make sure Config/Needs/verdepcheck and pre-commit config are updated
  • pre-release: Update verdpecheck and pre-commit #683

Update and Apply the Latest Roxygen Documentation Tags

1. Pre-release activities - Update and apply the latest roxygen documentation tags #647

Unused functions

• Pre-release activities - Unused functions #642
• Identify internal functions that are not used (if they exist)
• Discuss case by case if they should be removed

Replace val_labels with col_labels

• replace var_labels with teal.data::col_labels #658

Correct linting

• Adds common @return for modules and corrects linting #670

Standardize option Notation

• Standardize the notation for optional parameters in documentation #674

[m7pr]

Hey @insightsengineering/nest-core-dev based on the flow I've observed during other pre-release activities in our packages I allowed myself to divide this card into smaller activities

• Package level checks
• Vignettes level checks
• Exported functions level checks
• Non-exported functions level checks

I hope this will allow more people to be included in the collaboration in this process and will make the review process easier. Please make all PRs to pre-release@main branch, but please do not merge, even when a PR is reviewed and approved. We will merge everything, the moment all PRs to pre-release@main branch are approved and pre-release@main is also approved. My proposition is to have a pre-release@main branch be a branch where we review Package-level checks and then merge all other activities in there, before merging to main.

[averissimo]

Agree! It will require some extra focus when doing exported/non-exported, but I think this will help the review process greatly

[chlebowa]

While looking around for unused functions, I came upon add_facet_labels which is exported but not necessarily should be. Should we look for cases like that as well? Do we have time for such an assessment?

[m7pr]

@chlebowa maybe it's a good comment for #659 ?
If function is not used in any teal package nor in teal gallery it does not need to be exported.

It is used only in teal.modules.general in tm_g_bivariate so prefixing should be enough and no need for exporting. Examples can be extended for this function with getFromNamespace() (and potentially be removed for CRAN release)

[chlebowa]

Prefixing is only for foreign functions. This function (looks to me) is defined in tmg and only used in tmg functions with no apparent exposure to a teal app developer. A function like that should not be exported.

But perhaps there is another reason for this particular function to be exported?

[m7pr]

Yeah, I think you can remove the export. No need for prefixing if it's from tmg, yes, sorry! And only the examples need to be updated as it will not be exported anymore.

[averissimo]

Agree! How did you find this function? Is it an easy process or heavily manual?

[chlebowa]

I was manually going through all functions and searching their names to see whether and when they are called.

[m7pr]

For the automation, maybe there is a way you can list of functions and then grep it's appearance in a directory that contains all teal packages and teal,gallery. And sort functions by the number of apperances, so that the least frequent one can be examined manually?

[kartikeyakirar]

From the namespace file, we can directly check all the exported functions such as add_facet_labels and get_scatterplotmatrix_stats. However, both of these functions are exported and should be internal. I checked this by searching for these functions on the Github search bar for search in organization .

[chlebowa]

I did use the GH search bar to see if it's used in different packages but that is never the first place that I go. It ignores special characters and chops up queries on white space.

[donyunardi]

All works are completed.
Closing,