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

Tag and check for release candidates #61

Open
wants to merge 45 commits into
base: main
Choose a base branch
from
Open

Conversation

pat-s
Copy link
Contributor

@pat-s pat-s commented Oct 2, 2020

fixes #60

  • Creates RC tags in release()
  • Aborts the build if the build is running in "state = release" and the sha of the most recent commit == sha of the most recent tag && "-rc" is found in the most recent tag name
  • Deletes all RC tags in post_release()
  • Uses gh (if avail) to merge a PR instead of merging the branch.

ToDo

  • Enumerate RC tags because we can have multiple rejections

@pat-s pat-s requested a review from krlmlr October 2, 2020 14:01
@codecov
Copy link

codecov bot commented Oct 2, 2020

Codecov Report

Merging #61 into prerelease-state will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@                Coverage Diff                @@
##           prerelease-state     #61    +/-   ##
=================================================
  Coverage              0.00%   0.00%            
=================================================
  Files                    17      21     +4     
  Lines                   184     531   +347     
=================================================
- Misses                  184     531   +347     
Impacted Files Coverage Δ
R/api-bump-version.R 0.00% <0.00%> (ø)
R/api-finalize-version.R 0.00% <0.00%> (ø)
R/api-tag-version.R 0.00% <0.00%> (ø)
R/auto.R 0.00% <0.00%> (ø)
R/bump-version.R 0.00% <0.00%> (ø)
R/commit-version.R 0.00% <0.00%> (ø)
R/finalize-version.R 0.00% <0.00%> (ø)
R/gh-helpers.R 0.00% <0.00%> (ø)
R/state.R 0.00% <0.00%> (ø)
R/tag-version.R 0.00% <0.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19e1927...dedc3cf. Read the comment docs.

@pat-s
Copy link
Contributor Author

pat-s commented Oct 6, 2020

@krlmlr Successfully used this branch to release mlr v2.18.0 including to failed attempt (which triggered two "-rc" tags).

Copy link
Contributor

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Thanks. Can you please isolate the changes so that they apply directly to the f-27-automate branch, and open a new PR? I'm not ready to commit to state management, RC tagging is useful in itself.

R/auto.R Outdated
# FIXME: clean up mess

# check if gh is avail
is_gh_avail <- suppressWarnings(try(system2("gh", stdout = FALSE, stderr = FALSE), silent = TRUE))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather discuss this change in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Separate = third.

@krlmlr krlmlr marked this pull request as draft April 18, 2021 07:26
@pat-s pat-s changed the base branch from prerelease-state to f-27-automate April 18, 2021 09:26
R/auto.R Outdated
@@ -64,10 +64,12 @@ pre_release_impl <- function(which, force) {
# push main branch, bump to devel version and push again
push_to_new(remote_name, force)
switch_branch(main_branch)
push_head(main_branch)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if these two push_head() calls are still needed - guess we need to check in action

@pat-s
Copy link
Contributor Author

pat-s commented Apr 18, 2021

Rebased and stripped the diff, not sure if I removed something important though.

I'd just check how it behaves in practice then but ofc this is your decision in the end.

@pat-s pat-s requested a review from krlmlr April 18, 2021 10:03
@pat-s pat-s marked this pull request as ready for review April 18, 2021 10:03
@krlmlr
Copy link
Contributor

krlmlr commented Apr 18, 2021

/style

Merge back PR target branch
@krlmlr
Copy link
Contributor

krlmlr commented Apr 18, 2021

/merge

@pat-s
Copy link
Contributor Author

pat-s commented Jun 1, 2021

Did your latest comment have the intention to merge this PR?

@krlmlr
Copy link
Contributor

krlmlr commented Jun 4, 2021

No.

@krlmlr
Copy link
Contributor

krlmlr commented Jun 4, 2021

/merge

@krlmlr
Copy link
Contributor

krlmlr commented Jun 4, 2021

I'm experimenting with a flow that allows seamless integration of the target branch into the source branch. I do wonder why GitHub hasn't implemented this yet. Anyway, this is now available through the "/ merge" command in pull requests. Because GHA won't run checks when I push directly, I open a PR instead so that the user can merge manually and trigger checks in the target branch.

CC @wael-sadek.

@pat-s
Copy link
Contributor Author

pat-s commented Dec 11, 2021

@krlmlr How do you want to proceed here? I assume closing?

@krlmlr
Copy link
Contributor

krlmlr commented Dec 12, 2021

I'm open to discussing release candidate tags. About to merge that long-running PR, this will probably close this PR.

Base automatically changed from f-27-automate to main February 22, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants