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

Update DESCRIPTION #175

Merged
merged 3 commits into from
Oct 16, 2023
Merged

Update DESCRIPTION #175

merged 3 commits into from
Oct 16, 2023

Conversation

pawelru
Copy link
Contributor

@pawelru pawelru commented Oct 10, 2023

  • Config/Needs/verdepcheck entries in new line
  • add bioc:: for packages from Bioconductor

- Config/Needs/verdepcheck entries in new line
- add `bioc::` for packages from Bioconductor

Signed-off-by: Pawel Rucki <[email protected]>
@pawelru pawelru added the core label Oct 10, 2023
@pawelru pawelru requested a review from averissimo October 10, 2023 10:33
@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2023

badge

Code Coverage Summary

Filename                                 Stmts    Miss  Cover    Missing
-------------------------------------  -------  ------  -------  -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/as_cdisc.R                                39       4  89.74%   99-102
R/Callable.R                                45       0  100.00%
R/CallableCode.R                            36       2  94.44%   26, 63
R/CallableFunction.R                        88       3  96.59%   160-162
R/CallablePythonCode.R                      58      58  0.00%    21-227
R/cdisc_data.R                              45       1  97.78%   76
R/CDISCTealDataConnector.R                  20       3  85.00%   31, 36, 49
R/CDISCTealDataset.R                        46      11  76.09%   108-115, 204-206
R/CDISCTealDatasetConnector.R               26       1  96.15%   116
R/CodeClass.R                              111       1  99.10%   157
R/data_label.R                              36      13  63.89%   33-37, 56-63, 103
R/deep_clone_r6.R                            9       0  100.00%
R/dummy_function.R                           5       1  80.00%   23
R/formatters_var_labels.R                   49      21  57.14%   30, 38, 43-44, 46, 53, 99, 129-151
R/get_attrs.R                                2       2  0.00%    12-45
R/get_code.R                               173      19  89.02%   87, 140-143, 196-197, 207-208, 266, 297, 333, 337, 372, 381-385
R/get_dataname.R                             4       0  100.00%
R/get_dataset_label.R                        3       0  100.00%
R/get_dataset.R                             13       8  38.46%   39, 55, 80-86
R/get_datasets.R                            10       2  80.00%   96, 116
R/get_key_duplicates.R                      37       7  81.08%   41-47, 54-55
R/get_keys.R                                15       7  53.33%   67-68, 125-145
R/get_raw_data.R                            24      11  54.17%   168-181
R/include_css_js.R                           9       1  88.89%   20
R/is_pulled.R                                4       0  100.00%
R/JoinKeys.R                               183       7  96.17%   134, 233, 327, 330, 388-425
R/load_dataset.R                            25      18  28.00%   58-63, 87-186
R/MAETealDataset.R                         138      57  58.70%   53, 115, 153-208, 224-229, 236-245, 282, 323-339
R/mutate_dataset.R                          18       0  100.00%
R/set_args.R                                10       5  50.00%   34-38
R/teal_data.R                               31       2  93.55%   44, 52
R/TealData.R                               228     113  50.44%   206, 218-287, 330-337, 370-375, 377, 379-384, 386, 403-448
R/TealDataAbstract.R                       232      24  89.66%   72, 85-88, 97-106, 215-218, 429, 454-458, 480, 486
R/TealDataConnection.R                     297     180  39.39%   58-59, 64, 67, 70, 106-163, 183, 186-188, 194-200, 205-207, 233, 238, 254-277, 287, 300, 321, 325-330, 333-336, 358-360, 364-371, 374-377, 392-406, 425-426, 446-517, 535-543, 545, 549-564, 567-570, 602, 608-612, 626, 661-663, 672-674
R/TealDataConnector.R                      196     102  47.96%   178, 190, 194, 207, 210-219, 221, 229-238, 321-325, 383-488
R/TealDataset.R                            367      23  93.73%   141-151, 382-386, 442-451, 503
R/TealDatasetConnector_constructors.R      270      52  80.74%   177-214, 727-732, 930-1006
R/TealDatasetConnector.R                   326      90  72.39%   169, 237, 251, 256, 270, 433, 456-495, 525, 540-570, 660, 670, 679-686, 699, 714-741
R/to_relational_data.R                      54       7  87.04%   35-36, 40, 99, 106, 112, 127
R/topological_sort.R                        32       0  100.00%
R/utils.R                                   56       9  83.93%   22-23, 27, 76-83
R/validate_data_args.R                      32       0  100.00%
R/zzz.R                                      6       6  0.00%    4-12
TOTAL                                     3408     871  74.44%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: 520d737

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2023

Unit Tests Summary

       1 files       27 suites   18s ⏱️
   365 tests    365 ✔️ 0 💤 0
1 010 runs  1 010 ✔️ 0 💤 0

Results for commit 520d737.

♻️ This comment has been updated with latest results.

@pawelru pawelru enabled auto-merge (squash) October 11, 2023 09:17
Copy link
Contributor

@averissimo averissimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this.

The r.pkg.template/.github/workflows/version-bump.yaml action will always refactor the Config/Needs/verdepcheck to single line though (with NORMALIZE = TRUE option).

note: scheduled action was triggered manually and runs successfully: https://github.com/insightsengineering/teal.data/actions/runs/6533445688/job/17738647791

@pawelru
Copy link
Contributor Author

pawelru commented Oct 16, 2023

The r.pkg.template/.github/workflows/version-bump.yaml action will always refactor the Config/Needs/verdepcheck to single line though (with NORMALIZE = TRUE option).

Good find! Can you please make a PR then?

@pawelru pawelru merged commit a0f98b6 into main Oct 16, 2023
@pawelru pawelru deleted the pawelru-patch-1 branch October 16, 2023 13:02
@averissimo
Copy link
Contributor

Sure can, but @pawelru this will have a side effect that packages within fields won't be sorted or whitespace being "normalized". That is a nice feature to have.

We correct the desc_field during the gh action after normalizing

diff --git a/.github/workflows/version-bump.yaml b/.github/workflows/version-bump.yaml
index 0d41874..a8941a1 100644
--- a/.github/workflows/version-bump.yaml
+++ b/.github/workflows/version-bump.yaml
@@ -63,7 +63,23 @@ jobs:
 
       - name: Bump version in DESCRIPTION 📜
         if: ${{ (github.event_name == 'push' && github.ref == 'refs/heads/main') || inputs.vbump-after-release == true }}
-        run: desc::desc_bump_version("dev", normalize = TRUE)
+        run: |
+          desc::desc_bump_version("dev", normalize = TRUE)
+          # verdepcheck with 1 line per package
+          d <- desc::description$new("dev")
+          desc_field_name <- "Config/Needs/verdepcheck"
+          if (d$has_fields(field_name) && !identical(d$get_field(field_name), "")) {
+            desc_field_sep <- "\n    "
+            desc_field_normalized <- paste0(
+              field_sep,
+              paste(
+                strsplit(d$get_field(field_name), ", ")[[1]],
+                collapse = field_sep
+              )
+            )
+            d$set(field_name, desc_field_normalized)
+            d$write(file = "dev")
+          }
         shell: Rscript {0}
         working-directory: ${{ inputs.package-subdirectory }}
 

@pawelru
Copy link
Contributor Author

pawelru commented Oct 17, 2023

Thanks for checking. That's quite a lot of custom code :/ Let's leave that then.
Is this a worth raising within desc package? WDYT?

@averissimo
Copy link
Contributor

That would be best, I'll raise that issue

@averissimo
Copy link
Contributor

I managed to reduce the code to fewer lines. There was already an issue being tracked on r-lib/desc#122

diff --git a/.github/workflows/version-bump.yaml b/.github/workflows/version-bump.yaml
index 0d41874..f258d28 100644
--- a/.github/workflows/version-bump.yaml
+++ b/.github/workflows/version-bump.yaml
@@ -63,7 +63,14 @@ jobs:
 
       - name: Bump version in DESCRIPTION 📜
         if: ${{ (github.event_name == 'push' && github.ref == 'refs/heads/main') || inputs.vbump-after-release == true }}
-        run: desc::desc_bump_version("dev", normalize = TRUE)
+        run: |
+          desc::desc_bump_version("dev", normalize = TRUE)
+          # verdepcheck splitting into new lines
+          d <- desc::description$new("dev")
+          desc_field_name <- "Config/Needs/verdepcheck"
+          if (desc$has_fields(desc_field_name)) {
+            desc$set_list(desc_field_name, desc$get_list(desc_field_name), sep = ",\n\t\t")
+          }
         shell: Rscript {0}
         working-directory: ${{ inputs.package-subdirectory }}

Example

tf <- tempfile()
text <- "Package: test\nImports: tdplyr, tibble\nConfig/Needs/website: tidyverse/tidytemplate, r-lib/desc"

desc <- desc::description$new(text = text)
desc$normalize()
desc_field_name <- "Config/Needs/website"
if (desc$has_fields(desc_field_name)) {
  desc$set_list(desc_field_name, desc$get_list(desc_field_name), sep = ",\n    ")
}
desc$write(file = tf)
readLines(tf) |> paste0(collapse = "\n") |> cat()
#> Package: test
#> Imports: 
#>     tdplyr,
#>     tibble
#> Config/Needs/website: tidyverse/tidytemplate,
#>     r-lib/desc

Created on 2023-10-25 with reprex v2.0.2

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.

2 participants