Skip to content

Add check_mac_release function to submit to the mac builder#2387

Merged
jimhester merged 2 commits intomainfrom
check-mac
Nov 24, 2021
Merged

Add check_mac_release function to submit to the mac builder#2387
jimhester merged 2 commits intomainfrom
check-mac

Conversation

@jimhester
Copy link
Copy Markdown
Member

Fixes #2375

Comment thread R/check-mac.R Outdated

pkg <- as.package(pkg)

if (!is.null(email)) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The mac builder doesn't seem to use the email or send you results via email, so it is possible we should just remove this functionality for this. What do you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, just remove I think.

Comment thread R/check-mac.R Outdated
#' @family build functions
#' @return The url with the check results (invisibly)
#' @export
check_mac_release <- function(pkg = ".", args = NULL, manual = TRUE, email = NULL, quiet = FALSE, ...) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Currently it seems the only thing the public API allows is to check on R-release and the M1 macs. Though the response from the POST request indicates future plans to support more versions of R and architectures, e.g.

> httr::content(res)
$submitted
[1] TRUE

$id
[1] "1637073137-c5c2e7a4dcbedebf"

$package
[1] "glue"

$version
[1] "1.5.0.9000"

$rflavor
[1] "r-release"

$rfw
[1] "4.1-arm64"

$url
[1] "https://mac.R-project.org/macbuilder/results/1637073137-c5c2e7a4dcbedeb
f/"

Comment thread R/check-mac.R Outdated
encode = "multipart"
)

httr::stop_for_status(res)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you think we need a custom error message here, or is the default error for a bad status sufficient?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could use task = "Uploading package" as a compromise

Comment thread R/check-mac.R Outdated
time <- strftime(Sys.time() + 30 * 60, "%I:%M %p")

cli::cli_alert_success(
"[{Sys.Date()}] Check {.url {response_url}} for a link to results in 15-30 mins (~{time})."
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This time was based on win-builder times, it seems conservative for the typical mac-builder times, should we leave it or change it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think change it.

If the times are pretty short, is it worth adding a polling mode?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess the other thing is the results page seems to auto-refresh itself, so we could alternatively just use utils::browseURL() to open it for the user? (maybe only if it is interactive and !quiet?)

@jimhester jimhester requested a review from hadley November 16, 2021 15:06
@jimhester
Copy link
Copy Markdown
Member Author

I haven't yet added support for custom additional dependencies, but this is a first pass at check_mac_release().

- Removed the email argument
- added support for custom dependencies
- added `task` argument to `stop_for_status()`
Copy link
Copy Markdown
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

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

Sorry, meant to approve earlier.

@jimhester jimhester merged commit d247d52 into main Nov 24, 2021
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.

check_mac_devel()

2 participants