Skip to content

Conversation

@nikolas-burkoff
Copy link
Contributor

@nikolas-burkoff nikolas-burkoff commented Oct 11, 2022

Also styles the package

@cicdguy if you want to release this and update gh action then I'm happy to - I think the release process is manually change the version in DESC and update NEWS - then push tag beginning and it'll release.

We could do with the updated pkgdown to be built as well and to sync the renv.lock with the CI image ?

dplyr::select(.data$package_name, .data$cache_dir, .data$actions, .data$sha, .data$installable)
pkg_df %>%
dplyr::arrange(.data$install_index) %>%
dplyr::select("package_name", "cache_dir", "actions", "sha", "installable")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed here

@cicdguy
Copy link
Contributor

cicdguy commented Oct 11, 2022

Yes, I'll release this update and also update the GitHub action to use the new version.

Copy link
Contributor

@cicdguy cicdguy left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2022

badge

Code Coverage Summary

Filename                   Stmts    Miss  Cover    Missing
-----------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------
R/caching.R                  167       5  97.01%   68, 131-134
R/dependencies_app.R         131     131  0.00%    38-193
R/dependencies_helper.R      238      72  69.75%   42-43, 47, 52-55, 91-107, 113-122, 127-134, 140-143, 148-155, 161, 169-175, 230, 264, 276-294, 305, 414
R/dependencies.R             347      42  87.90%   87, 94, 112, 130, 214-230, 273, 281-284, 286, 299, 384, 449, 494-507, 552, 623-624, 631, 722
R/git_tools.R                183      87  52.46%   55, 73, 92-99, 102, 107, 124, 135-138, 155, 158-165, 181-205, 245-321
R/graph_methods.R             71       1  98.59%   116
R/host.R                      19      10  47.37%   5-8, 19-25
R/ref_strategy.R              44       0  100.00%
R/renv.R                      13       0  100.00%
R/rstudio_addins.R             5       5  0.00%    6-22
R/rstudio_jobs.R              27      27  0.00%    3-74
R/utils.R                     96       6  93.75%   2, 8, 72-73, 82, 92
R/zzz.R                       18      10  44.44%   27-37
TOTAL                       1359     396  70.86%

Results for commit: 509b8fb

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@nikolas-burkoff
Copy link
Contributor Author

nikolas-burkoff commented Oct 12, 2022

I've updated the lock.file to latest packages (including tidyselect 1.2.0) and updated the version in the DESC file/NEWS

Locally we're looking good:

image

@pawelru
Copy link
Collaborator

pawelru commented Oct 14, 2022

So CI is failing - please fix them

Other than that, I logged into our internal env, did renv::deactivate() to use packages from there and ran tests*:

==> devtools::test()

ℹ Loading staged.dependencies
ℹ Testing staged.dependencies
Unpacking test ecosystem to directory /local/tmp/Rtmpm8OylB/test_ecosystem7d872cd7e2e8 
✔ | F W S  OK | Context
✔ |        23 | caching [0.9s]                                                                             
✔ |         6 | check_set_equal                                                                            
✖ | 4      28 | dependencies [4.9s]                                                                        
───────────────────────────────────────────────────────────────────────────────────────────────────────────
Error (test-dependencies.R:164:3): plot.dependency_structure works
Error in `parse(text = elt)`: <text>:1:5: unexpected symbol
1: Use of
        ^
Backtrace:
  1. plot.dependency_structure(dep_table) %>% ...
       at test-dependencies.R:164:2
 13. dplyr:::rename.data.frame(., to = .data$from, from = .data$to)
 14. tidyselect::eval_rename(expr(c(...)), .data)
 15. tidyselect:::rename_impl(...)
 16. tidyselect:::eval_select_impl(...)
 20. tidyselect:::vars_select_eval(...)
 21. tidyselect:::walk_data_tree(expr, data_mask, context_mask)
 22. tidyselect:::eval_c(expr, data_mask, context_mask)
 23. tidyselect:::reduce_sels(node, data_mask, context_mask, init = init)
     ...
 28. lifecycle::signal_stage("deprecated", what)
 29. lifecycle:::spec(what, env = env)
 30. lifecycle:::spec_what(spec, "spec", signaller)
 31. rlang::parse_expr(what)
 32. rlang::parse_exprs(x)
 33. rlang:::chr_parse_exprs(x)
 34. rlang:::map(x, function(elt) as.list(parse(text = elt)))
 35. base::lapply(.x, .f, ...)
 36. rlang (local) FUN(X[[i]], ...)
 38. base::parse(text = elt)

Error (test-dependencies.R:188:3): install_deps works
Error in `dplyr::select(., .data$package_name, .data$cache_dir, .data$actions, 
    .data$sha, .data$installable)`: <text>:1:5: unexpected symbol
1: Use of
        ^
Backtrace:
 1. testthat::expect_equal(...)
      at test-dependencies.R:188:2
 8. dplyr:::select.data.frame(...)

Error (test-dependencies.R:246:3): check_downstream works
Error in `dplyr::select(., .data$package_name, .data$cache_dir, .data$actions, 
    .data$sha, .data$installable)`: <text>:1:5: unexpected symbol
1: Use of
        ^
Backtrace:
 1. testthat::expect_equal(...)
      at test-dependencies.R:246:2
 8. dplyr:::select.data.frame(...)

Error (test-dependencies.R:280:3): build_check_install works
Error in `dplyr::select(., .data$package_name, .data$cache_dir, .data$actions, 
    .data$sha, .data$installable)`: <text>:1:5: unexpected symbol
1: Use of
        ^
Backtrace:
 1. testthat::expect_equal(...)
      at test-dependencies.R:280:2
 8. dplyr:::select.data.frame(...)
───────────────────────────────────────────────────────────────────────────────────────────────────────────
✔ |        23 | dependencies_helper [0.2s]                                                                 
✔ |        14 | get_descendants_distance                                                                   
✔ |         3 | git_tools                                                                                  
✔ |        11 | git_tools_mocking [0.2s]                                                                   
✔ |        21 | graph_methods                                                                              
✔ |        18 | ref_strategy [0.1s]                                                                        
✔ |         8 | renv                                                                                       
✔ |        14 | utils                                                                                      
✔ |        18 | validate_yaml [0.1s]                                                                       
✔ |         5 | zzz                                                                                        

══ Results ════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 6.8 s

[ FAIL 4 | WARN 0 | SKIP 0 | PASS 192 ]
  • Note that I needed to comment-out checkout_repo tests as it got stuck on it.

@nikolas-burkoff
Copy link
Contributor Author

nikolas-burkoff commented Oct 14, 2022

So @cicdguy is working on the CI at the moment (it's to do with the action).

@pawelru
Copy link
Collaborator

pawelru commented Oct 14, 2022

OMG sorry - I was testing on main. It seems that all is good on your branch.

Copy link
Collaborator

@pawelru pawelru left a comment

Choose a reason for hiding this comment

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

Once CI is fixed it's good to go

@nikolas-burkoff
Copy link
Contributor Author

@cicdguy - just making sure this doesn't drop off our radar

@cicdguy cicdguy merged commit becf5cb into main Dec 14, 2022
@cicdguy cicdguy deleted the tidy_select@main branch December 14, 2022 12:00
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.

4 participants