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

v1.7.5.1 release candidate #3910

Merged
merged 3 commits into from
Oct 16, 2023
Merged

v1.7.5.1 release candidate #3910

merged 3 commits into from
Oct 16, 2023

Conversation

cpsievert
Copy link
Collaborator

@cpsievert cpsievert commented Oct 5, 2023

Closes #3911.

This patch release branches off the v1.7.5 tag to only include #3908 (because CRAN has requested a fix by Oct 14 and we're not in a position to release the rest of what's on main).

@cpsievert cpsievert force-pushed the rc-v1.7.5.1 branch 2 times, most recently from 90f0bc5 to 2f434ff Compare October 5, 2023 23:04
@cpsievert
Copy link
Collaborator Author

cpsievert commented Oct 6, 2023

Note that 837307f has a false positive error because the routine is being overly opinionated about using -alpha in the package.json (npm doesn't support 4 digits, so x.y.z-foo is a way to workaround that, but -alpha is not required, and since this is an official release, we really shouldn't be including -alpha in the version)

837307f was submitted to CRAN and is awaiting response

@rhenkin
Copy link

rhenkin commented Oct 13, 2023

Hi, do you have any updates on this? I have a package that calls isTruthy and that will be removed from CRAN tomorrow, but if this goes through I won't need to change anything (possibly just re-submit if they don't accept the new shiny version on time).

Thanks,
Rafael

@wch
Copy link
Collaborator

wch commented Oct 13, 2023

We submitted it to CRAN a week ago, and their automatic reverse dependency checks had an error in the vignette for a package. I think that error had nothing to do with Shiny, and I emailed CRAN twice about it, but still haven't heard back from them.

@wch
Copy link
Collaborator

wch commented Oct 13, 2023

This is the relevant part of the email from CRAN:

CRAN teams' auto-check service
Package check result: OK

Changes to worse in reverse depends:

Package: hyd1d
Check: re-building of vignette outputs
New result: ERROR
  Error(s) in re-building vignettes:
    ...
  --- re-building ‘hyd1d.Rmd’ using rmarkdown
  --- finished re-building ‘hyd1d.Rmd’

  --- re-building ‘vignette_DE.Rmd’ using rmarkdown

  Quitting from lines 1165-1190 [figure15] (vignette_DE.Rmd)
  Error: processing vignette ‘vignette_DE.Rmd’ failed with diagnostics:
  the condition has length > 1
  --- failed re-building ‘vignette_DE.Rmd’

  SUMMARY: processing the following file failed:
    ‘vignette_DE.Rmd’

  Error: Vignette re-building failed.
  Execution halted

The error is apparently from this code block:
https://github.com/bafg-bund/hyd1d/blob/release-v0.4.6/vignettes/vignette_DE.Rmd#L1165-L1189

@gadenbuie
Copy link
Member

@wch I installed r-devel (latest available via rig was 2023-10-09 r85295) and using this version of Shiny, I could not reproduce the problem.

However! I noticed that the functions in that vignette collect data from remote online resources. By disconnecting my computer from the internet, I was able to reproduce the exact error, and the output gives a little more context:

> rmarkdown::render("vignettes/vignette_DE.Rmd")


processing file: vignette_DE.Rmd
  |...                                      |   8% [captions]                  curl: (6) Could not resolve host: hyd1d.bafg.de
  |.................................        |  79% [figure15]
Quitting from lines 1165-1190 [figure15] (vignette_DE.Rmd)
Error in `if (length(s_gauging_stations_missing) == 1 & is.na(s_gauging_stations_missing)) ...`:
! the condition has length > 1
Backtrace:
 1. base::summary(wldf)
 2. hyd1d:::summary.WaterLevelDataFrame(wldf)

@wch
Copy link
Collaborator

wch commented Oct 15, 2023

@rhenkin Shiny 1.7.5.1 is on CRAN now, so you should be good to go.

@cpsievert cpsievert marked this pull request as ready for review October 16, 2023 14:58
@cpsievert
Copy link
Collaborator Author

@wch this PR is mergeable now -- I think we'll want to do a merge commit (not squash+merge) so that the v1.7.5.1 tag (837307f) gets properly included into main's history?

Also, it appears the routine failure in 481a692 is, again, due to shiny-workflows wanting to use the rc-v* branch name to inform the version

@wch
Copy link
Collaborator

wch commented Oct 16, 2023

Yes, I think we're good to merge this now.

One thing that I'm a little confused by is that the Files Changed view in this PR only shows that the NEWS.md file has changed. Is that because main also contained the fix, and main was merged in to this branch?

@cpsievert
Copy link
Collaborator Author

Is that because main also contained the fix, and main was merged in to this branch?

Yep, this PR started by branching off of the v1.7.5 tag to only include essentially the latest commit to main. And now that it's been merged with main, the only changes should be to the NEWS.md

@cpsievert cpsievert merged commit 80ab088 into main Oct 16, 2023
22 of 23 checks passed
@cpsievert cpsievert deleted the rc-v1.7.5.1 branch October 16, 2023 17:22
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.

Release shiny 1.7.5.1
4 participants