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 workflows #137

Merged
merged 6 commits into from
Apr 23, 2024
Merged

update workflows #137

merged 6 commits into from
Apr 23, 2024

Conversation

pawelru
Copy link
Contributor

@pawelru pawelru commented Apr 8, 2024

I'm going to trigger verdepcheck pipelines manually. You can check it's status here: https://github.com/insightsengineering/osprey/actions/workflows/scheduled.yaml?query=branch%3Afix_verdepcheck

Copy link
Contributor

github-actions bot commented Apr 8, 2024

badge

Code Coverage Summary

Filename                   Stmts    Miss  Cover    Missing
-----------------------  -------  ------  -------  ---------
R/g_ae_sub.R                 344     344  0.00%    147-536
R/g_butterfly.R              183     183  0.00%    102-319
R/g_events_term_id.R         263     263  0.00%    129-490
R/g_heat_bygrade.R           210     210  0.00%    139-371
R/g_hy_law.R                  77      77  0.00%    119-199
R/g_patient_profile.R        390     390  0.00%    295-959
R/g_spiderplot_simple.R       20      20  0.00%    37-59
R/g_spiderplot.R             174     174  0.00%    109-345
R/g_swimlane.R               139     139  0.00%    119-320
R/g_waterfall.R              306     306  0.00%    146-506
R/stream_filter.R            126     126  0.00%    34-296
R/utils.R                    153     153  0.00%    43-362
TOTAL                       2385    2385  0.00%

Diff against main

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

Results for commit: 0da45f9

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@averissimo averissimo self-assigned this Apr 22, 2024
@averissimo
Copy link
Contributor

averissimo commented Apr 22, 2024

Updated versions and section on DESCRIPTION.

Workflow is running, I'll be fixing it until badge is green or all fails are "expected"

Scheduled 🕰️

(this badge is for the fix_verdepcheck branch)

@averissimo averissimo marked this pull request as draft April 22, 2024 12:12
@averissimo
Copy link
Contributor

This line with guide-box-bottom added by #131 requires latest ggplot2 (3.5.0)

mylegend <- grob_part(grob_part(p1_parts, "guide-box-bottom"), "guides")

@averissimo
Copy link
Contributor

@pawelru I'm wondering if there is any reason why ggplot2@3.5.0 may not be used here?

Such as: ggplot2 not being available in a production/testing environment?

@averissimo averissimo marked this pull request as ready for review April 22, 2024 14:45
@pawelru
Copy link
Contributor Author

pawelru commented Apr 22, 2024

@pawelru I'm wondering if there is any reason why ggplot2@3.5.0 may not be used here?

Such as: ggplot2 not being available in a production/testing environment?

I am probably missing some context here. What are you referring to?

@averissimo
Copy link
Contributor

Sorry, for the lack of context.

The minimum strategies were failing as they need gggplot2@3.5.0 since #131

My concrete question:

  • Do you anticipate a problem by bumping ggplot2 to version 3.5.0 from February 2024?
  • Namely from environments where this package may be tested/deployed

@pawelru
Copy link
Contributor Author

pawelru commented Apr 23, 2024

Ahh it's a pitty I haven't added this to my PR. Sorry about that.

If I recall correctly, ggplot2 throws a bunch of lifecycle warnings that some functions have been softly deprecated and there are fully functional alternatives. Looking then into the PR it seems that it's about aes_string() and size argument of geom_segment(). An evidence in the bottom of my message. These were also present in our CI that automatically build testing environment with the newest version of CRAN packages.


r$> aes_string()
Aesthetic mapping: 
<empty>
Warning message:
`aes_string()` was deprecated in ggplot2 3.0.0.
ℹ Please use tidy evaluation idioms with `aes()`.
ℹ See also `vignette("ggplot2-in-packages")` for more information.
This warning is displayed once every 8 hours.
Call `lifecycle::last_lifecycle_warnings()` to see where this warning was generated.
r$> geom_segment(size = 0.5)
geom_segment: arrow = NULL, arrow.fill = NULL, lineend = butt, linejoin = round, na.rm = FALSE
stat_identity: na.rm = FALSE
position_identity 
Warning message:
Using `size` aesthetic for lines was deprecated in ggplot2 3.4.0.
ℹ Please use `linewidth` instead.
This warning is displayed once every 8 hours.
Call `lifecycle::last_lifecycle_warnings()` to see where this warning was generated.

@pawelru
Copy link
Contributor Author

pawelru commented Apr 23, 2024

It looks like all the verdepcheck pipelines are green - last run. You have correctly bumped up ggplot dependency. This is not a problem anymore. Am I right on this?

@averissimo
Copy link
Contributor

Yup! this one is ready to be merged as soon as CI workflow starts to works as expected

@pawelru pawelru enabled auto-merge (squash) April 23, 2024 09:12
@pawelru pawelru merged commit 3129c10 into main Apr 23, 2024
24 checks passed
@pawelru pawelru deleted the fix_verdepcheck branch April 23, 2024 11:16
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.

2 participants