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

[3.4] fix release.sh: git_assert_branch_in_sync not exist in 3.4 #15715

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Apr 14, 2023

The git_assert_branch_in_sync doesn't exist in 3.4, so let's just remove it.

The issue was introduced in 49d05f8. In our workflow, it always run in in-place mode, so it never triggers the branch.

Signed-off-by: Benjamin Wang <wachao@vmware.com>
@ahrtr
Copy link
Member Author

ahrtr commented Apr 14, 2023

cc @serathius PTAL

@serathius
Copy link
Member

serathius commented Apr 14, 2023

Should we backport git_assert_branch_in_sync as it seems useful? Seems like we just forgot to backport it.

@@ -6,7 +6,7 @@ source ./scripts/test_lib.sh

VER=$1
PROJ="etcd"
REPOSITORY="${REPOSITORY:-git@github.com:etcd-io/etcd.git}"
REPOSITORY="${REPOSITORY:-https://github.com/etcd-io/etcd.git}"
Copy link
Member

Choose a reason for hiding this comment

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

Note that also applies on other branches. Not as important for this PR.

I prefer git@github.com:etcd-io/etcd.git as during push I can depend on SSH security instead of typing in credentials. It's much safer. Best would be to take address of origin remote from original repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. But in 3.4, actually I never depend on the automatically git push. Instead, each time, I manually bump the version and create/push the tag.

Let's just keep it as it was. It was working well previous. After the 49d05f8, it broke the 3.4 release, so I tend to partially revert the change.

We can consider to enhance it later; but I don't expect to make much big change on 3.4.

@ahrtr
Copy link
Member Author

ahrtr commented Apr 14, 2023

thx @serathius

I will not allow any release script change anymore in future, so as not to break it again. Because usually our workflow/pipeline will never test the real release execution path.

@ahrtr ahrtr merged commit 94593e6 into etcd-io:release-3.4 Apr 14, 2023
@ahrtr ahrtr deleted the fix_release_20230414 branch June 12, 2024 11:40
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.

None yet

2 participants