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

Pre-release activities #506

Merged
merged 96 commits into from
Jan 25, 2024
Merged

Pre-release activities #506

merged 96 commits into from
Jan 25, 2024

Conversation

kartikeyakirar
Copy link
Contributor

@kartikeyakirar kartikeyakirar commented Dec 28, 2023

part of insightsengineering/nestdevs-tasks#48

  • Review and update:
    - README.md (check example code)
    - NEWS.md
  • Run urlchecker::url_check() to identify broken links and fix
  • 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
  • Sanity check of all vignettes, make sure there is no typo, no wrong format, etc.
  • 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 there are no ::: in examples
  • Make sure all teal.* mentions are lower-cased and quoted
  • Make Sure inst/WORDLIST is minimalized
  • Make sure non-exported functions do not have examples
  • 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
  • switch from title case into sentence case for title and description of functions.
  • remove old rd syntax

@kartikeyakirar kartikeyakirar marked this pull request as draft December 28, 2023 13:38
Copy link
Contributor

github-actions bot commented Dec 28, 2023

badge

Code Coverage Summary

Filename                        Stmts    Miss  Cover    Missing
----------------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------------------------
R/calls_combine_by.R                7       0  100.00%
R/choices_labeled.R                49      14  71.43%   25, 36, 41, 51-56, 68, 72-76
R/count_labels.R                   97       0  100.00%
R/filter_panel_api.R               29       1  96.55%   134
R/FilteredData-utils.R             68      25  63.24%   17-20, 23-26, 48-53, 146, 168-177
R/FilteredData.R                  554     218  60.65%   181, 323, 395, 502-511, 535, 556-597, 615-618, 634, 676-709, 725-727, 731-737, 764-792, 816-818, 822-824, 827-838, 842-851, 853-891, 932, 955-977
R/FilteredDataset-utils.R          23       1  95.65%   112
R/FilteredDataset.R               176      67  61.93%   49, 149, 192-198, 226-283, 322-324
R/FilteredDatasetDataframe.R      121       8  93.39%   82, 144, 154, 230-234
R/FilteredDatasetDefault.R         18       4  77.78%   94-107
R/FilteredDatasetMAE.R            134      37  72.39%   49, 110-115, 154-159, 163-164, 182-204
R/FilterPanelAPI.R                 10       0  100.00%
R/FilterState-utils.R             101       2  98.02%   256, 286
R/FilterState.R                   361      61  83.10%   86, 210, 228-232, 239-240, 254-255, 261-262, 310, 312, 314, 366, 410, 642, 685-710, 721-740, 775-781, 790-796
R/FilterStateChoices.R            338     106  68.64%   309-312, 324, 367, 391-398, 402-419, 448, 463-474, 486-494, 498-527, 548-551, 554-557, 568-589, 602-603, 613
R/FilterStateDate.R               212     129  39.15%   226, 279-437
R/FilterStateDatettime.R          309     199  35.60%   266, 319-551
R/FilterStateEmpty.R               53      31  41.51%   89, 99-104, 118, 132-173
R/FilterStateExpr.R                75      62  17.33%   143-267
R/FilterStateLogical.R            196     144  26.53%   135, 158, 218, 222-409
R/FilterStateRange.R              410     105  74.39%   260, 384, 512-516, 519-529, 532, 544-550, 561-573, 577-587, 591-593, 607-634, 649, 652, 667-684, 719-724, 734-736
R/FilterStates-utils.R             70       9  87.14%   106, 125, 185-191, 213, 242
R/FilterStates.R                  364      30  91.76%   78-82, 191, 317-326, 414-417, 460, 545-549, 594, 715-718
R/FilterStatesDF.R                  5       0  100.00%
R/FilterStatesMAE.R                10       1  90.00%   40
R/FilterStatesMatrix.R              3       0  100.00%
R/FilterStatesSE.R                211     157  25.59%   36, 71-73, 83-85, 109-116, 124-131, 154-302
R/include_css_js.R                  5       5  0.00%    12-16
R/teal_slice.R                    107       4  96.26%   134, 188-189, 200
R/teal_slices.R                    84       5  94.05%   146-151
R/test_utils.R                     21       0  100.00%
R/utils.R                          18       0  100.00%
R/variable_types.R                 48      33  31.25%   45-50, 60, 73-108
R/zzz.R                            16      16  0.00%    3-46
TOTAL                            4303    1474  65.74%

Diff against main

Filename                    Stmts    Miss  Cover
------------------------  -------  ------  --------
R/calls_combine_by.R           -1       0  +100.00%
R/count_labels.R               -1       0  +100.00%
R/filter_panel_api.R           -6       0  -0.59%
R/FilteredData-utils.R        -30      -4  -7.17%
R/FilterStateDatettime.R       +2       0  +0.42%
R/utils.R                      -7      -2  +8.00%
TOTAL                         -43      -6  -0.20%

Results for commit: 5ecbaa9

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Dec 28, 2023

Unit Tests Summary

  1 files   29 suites   22s ⏱️
359 tests 359 ✅ 0 💤 0 ❌
822 runs  822 ✅ 0 💤 0 ❌

Results for commit 5ecbaa9.

♻️ This comment has been updated with latest results.

@kartikeyakirar kartikeyakirar marked this pull request as ready for review January 2, 2024 11:15
R/FilterState-utils.R Show resolved Hide resolved
R/FilterStateChoices.R Show resolved Hide resolved
R/FilterStateDate.R Show resolved Hide resolved
R/FilterStateDatettime.R Show resolved Hide resolved
R/FilterStateEmpty.R Outdated Show resolved Hide resolved
vignettes/filter-panel-for-developers.Rmd Outdated Show resolved Hide resolved
vignettes/filter-panel-for-developers.Rmd Outdated Show resolved Hide resolved
vignettes/filter-panel-for-developers.Rmd Outdated Show resolved Hide resolved
vignettes/teal-slice-classes.Rmd Outdated Show resolved Hide resolved
vignettes/teal-slice-classes.Rmd Outdated Show resolved Hide resolved
kartikeyakirar and others added 7 commits January 2, 2024 21:37
Co-authored-by: Aleksander Chlebowski <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: Aleksander Chlebowski <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
R/FilterState.R Show resolved Hide resolved
R/FilterState.R Outdated Show resolved Hide resolved
R/FilterStateDate.R Outdated Show resolved Hide resolved
R/FilterStateExpr.R Outdated Show resolved Hide resolved
R/FilterStates-utils.R Outdated Show resolved Hide resolved
@m7pr
Copy link
Contributor

m7pr commented Jan 24, 2024

Hey @kartikeyakirar I left few minor comments, but I would like them to be resolved before we merge

@m7pr m7pr mentioned this pull request Jan 24, 2024
@m7pr
Copy link
Contributor

m7pr commented Jan 25, 2024

Hey, just to summarize what's going on during this PR.
@kartikeyakirar did a great job eliminating ~20 cleanup issues from the pre-release checklist.

Changes in repository included

  • addition of new CRAN badges to README
  • changing installation instructions to regular installation from CRAN
  • addition of the CRAN RELEASE issue template
  • cleanup of pkgdown titles

During this PR @chlebowa proposed we move internal examples to a special vignette (with an addition to documentation with @seealso tag pointing to this vignette). Later on we also discussed if we should move them to inst/demo() part of the package. At the end of the day, we decided to use getFromNamespace() function to overcome limitation of ::: usage in the documentation, which is discouraged by CRAN policy. The vignette was discarded.

Other main changes included

  • examples and vignettes were run and verified for compliteness
  • refinement of roxygen2 @title, @description and @details sections/tags
  • WORDLIST cleanup and typos verification
  • review of cleanness of the documentation
    • mostly classes of parameters
    • addition of @internal tags to some pages so they are not listed in the index of the package
    • quoting package names and R with backticks
  • removal of :: prefixing and library(shiny) calls from examples since shiny is in Depends and prefixing in examples clutters the picture
  • tests:
  • vignettes:
    • filter panel for developer vignettes had it's graph of dependencies refreshed
    • classes were renamed (e.g. DefaultFilteredDatase to DataframeFilteredDataset, FilteredDatasetMAE to MAEFilteredDataset) and their appearance in the proper place of the vignettes was reviewed

Final remarks

Multiple people were involved in this PR, kudos to @kartikeyakirar @chlebowa and @averissimo
A lot of things have been provided in this PR (112 files changed!) and I think it's time to merge this. DONE is better than PERFECT. We can tackle other small things outside of this PR. It can not grow any longer and I state it's in good shape to be merged.

@m7pr m7pr dismissed chlebowa’s stale review January 25, 2024 11:07

this is resolved!

@kartikeyakirar
Copy link
Contributor Author

Thank you, @m7pr, for your excellent summary of the PR and approving it. I concur that it contains numerous changes, and as you suggested any additional change requests should be separate issue. Reviewing the entire pull request in one go could be challenging. I plan to merge this pull request in 3 hours, which should provide enough time for any final comments or feedback.

thanks everyone.

closes #531 
We used to distinguish sub-items of the MAE object by term `datalabel`.
We don't do this anymore, this is artifact after the refactor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants