Skip to content

RELEASING: ~~first merge PR, then tag~~ clarify where to push the tag #7741

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

Merged
merged 1 commit into from
Sep 19, 2020

Conversation

bluetech
Copy link
Member

The previous order is not really possible to perform -- before merging there is no release commit in the MAJOR.MINOR.x branch.

@bluetech bluetech mentioned this pull request Sep 11, 2020
RELEASING.rst Outdated

#. Tag the merge commit in the ``MAJOR.MINOR.x`` branch and push it. This will publish to PyPI::
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I say to tag the merge commit but can also tag the release commit itself. The merge commit seems more appropriate to me, but I see previous releases tagged the release commit. Going to keep it as the merge commit but can change it.

@nicoddemus
Copy link
Member

nicoddemus commented Sep 11, 2020

The previous order is not really possible to perform -- before merging there is no release commit in the MAJOR.MINOR.x branch.

Actually it is, I've always done it that way: even thought upstream still doesn't contain the commit anywhere, by pushing a tag to upstream the commit is now part of it.

I really prefer this workflow, as we are tagging exactly the commit which was released; doing that after the merge is problematic, as in the time between the release branch being created and the actual release, other commits might have been merged.

@RonnyPfannschmidt
Copy link
Member

Dito, I agree with @nicoddemus

@bluetech
Copy link
Member Author

OK got it. But I figure we should still change "tag the release commit in the MAJOR.MINOR.x branch" to "in the release-MAJOR.MINOR.PATCH branch"?

@nicoddemus
Copy link
Member

nicoddemus commented Sep 12, 2020

OK got it. But I figure we should still change "tag the release commit in the MAJOR.MINOR.x branch" to "in the release-MAJOR.MINOR.PATCH branch"?

I find the former more precise (we tag commits, not branches), but perhaps "tag the commit from the build that published the release to PyPI (the HEAD of MAJOR.MNOR.x branch)"? WDYT?

@bluetech
Copy link
Member Author

I must be misunderstanding something but

tag the commit from the build that published the release to PyPI

It's the tag itself that publishes to PyPI so the causality is reversed here?

(the HEAD of MAJOR.MNOR.x branch)

Before merging the release PR, the HEAD of the MAJOR.MINOR.x branch is not the release commit, it is something before. At this point, the release commit is only found in the release-MAJOR.MINOR.PATCH branch. So the needed procedure is to checkout the release branch (the PR opened by pytestbot) and tag the commit there.

@nicoddemus
Copy link
Member

I'm sorry, I misspoke. 😬

Before merging the release PR, the HEAD of the MAJOR.MINOR.x branch is not the release commit, it is something before. At this point, the release commit is only found in the release-MAJOR.MINOR.PATCH branch. So the needed procedure is to checkout the release branch (the PR opened by pytestbot) and tag the commit there.

You are absolutely correct.

To be crystal clear:

$ git fetch --all
$ git tag 6.0.2 upstream/release-6.0.2
$ git push upstream 6.0.2

# now merge release-6.0.2 PR

# cheery-pick CHANGELOG changes from `upstream/MAJOR.MINOR.x` to `master`

Perhaps we can reword the instructions to suggest that developers have origin as their fork, upstream as pytest-dev/pytest, and update the instructions accordingly? I prefer this setup because usually tooling defaults to pushing to origin, which is very often the right thing to do.

@bluetech
Copy link
Member Author

@nicoddemus Updated per your comments.

Perhaps we can reword the instructions to suggest that developers have origin as their fork, upstream as pytest-dev/pytest, and update the instructions accordingly?

This is already mentioned at the beginning of the doc ("The git commands assume the following remotes are setup ...")

@bluetech bluetech changed the title RELEASING: first merge PR, then tag RELEASING: ~~first merge PR, then tag~~ clarify where to push the tag Sep 19, 2020
@bluetech bluetech merged commit 8eefe4e into pytest-dev:master Sep 19, 2020
@bluetech bluetech deleted the releasing-order branch October 1, 2020 14:20
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.

3 participants