-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: New init_release()
, to be called before pre_release()
#686
Conversation
94770c9
to
044544f
Compare
Editing the OP here to add everything that will be closed with this PR. |
R/auto.R
Outdated
# FIXME: Needs repair in create_release_branch() | ||
stopifnot(!force) | ||
init_release_impl <- function(which, force) { | ||
# FIXME: Slightly different process for releasing from the non-main branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
releasing from the non-main branch is undocumented, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should work, eventually. Also, bumping on the non-main branch should eventually work (but never delete tags).
R/auto.R
Outdated
branch_name <- paste0("cran-", desc::desc_get_version()) | ||
merge_dev_news <- function(fledgeling, new_version) { | ||
dev_idx <- grepl("^[0-9]+[.][0-9]+[.][0-9]+[.][0-9]+$", fledgeling$news$version) | ||
stopifnot(dev_idx[[1]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? add comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise, we would be counting the wrong thing in the rle()
call below. A comment would be great indeed, or perhaps extract a function?
R/bump-version.R
Outdated
@@ -33,7 +34,7 @@ bump_version_impl <- function(which, | |||
#' 1. Depending on the `which` argument: | |||
if (which == "dev") { | |||
#' - If `"dev"`, [finalize_version()] with `push = FALSE` | |||
finalize_version_impl(push = FALSE) | |||
finalize_version_impl(push = FALSE, suggest_finalize = edit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this argument name unclear. edit is clearer. could we rename suggest_finalize to edit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the argument name make more sense in the context of finalize_version_impl()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still find "edit" clearer
ae25da6
to
9d336e5
Compare
I tried adding cached responses for |
progress! https://github.com/maelle/tea/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc next time I'll look into why the responses aren't cached but at least I am able to create the toy repo 😁 |
The checks for the last commit asks for a |
I now also think that bumping the version with the special message should move to |
The list of user action items after |
|
Some notes on mocking:
|
Is a change here needed? |
a31c447
to
584222f
Compare
584222f
to
b333f8d
Compare
b333f8d
to
3c1ac46
Compare
pre_release()
init_release()
, to be called before pre_release()
Thanks! |
Decided to keep it closer to the current workflow. Added an
init_release()
that does part of whatpre_release()
is doing.@maelle: Can you please, starting at the beginning, pick up the commits one by one and add tests, perhaps in new individual PRs? I'll keep managing this branch.
pre_release()
#658.pre_release()
: Add action item to clean up NEWS.md from .9xxx entries #37.