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

(Breaking change) chore: change default target-branch to main #510

Merged
merged 4 commits into from
Nov 19, 2023
Merged

(Breaking change) chore: change default target-branch to main #510

merged 4 commits into from
Nov 19, 2023

Conversation

fty4
Copy link
Contributor

@fty4 fty4 commented Jan 6, 2023

What this PR does / why we need it:

Because the default branch on GitHub is main and not longer master this should also be the default to chart-testing.

Which issue this PR fixes:

fixes #376

Special notes for your reviewer:

BREAKING CHANGE:
If e.g an action or workflow is not using an explicit specified target-branch the job might will fail after this change.

But after this change new users does not have the problem that a wrong branch is used.
For example I was suffering with the error message exit status 128- see #330 because the default branch was not main.

@fty4 fty4 changed the title Chore/target branch main chore: change default target-branch to main Jan 6, 2023
Copy link

@thisisrachelramos thisisrachelramos left a comment

Choose a reason for hiding this comment

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

@cpanato cpanato closed this Jul 6, 2023
@cpanato cpanato reopened this Jul 6, 2023
@cpanato cpanato requested a review from davidkarlsen July 6, 2023 08:12
@cpanato
Copy link
Member

cpanato commented Jul 6, 2023

@fty4 can you rebase and update the docs? thanks for this PR

fty4 added 3 commits July 17, 2023 13:57
closes #376

Signed-off-by: Marco Lecheler <[email protected]>
Signed-off-by: Marco Lecheler <[email protected]>
Signed-off-by: Marco Lecheler <[email protected]>
Signed-off-by: Marco Lecheler <[email protected]>
Signed-off-by: Marco Lecheler <[email protected]>
Signed-off-by: Marco Lecheler <[email protected]>
@fty4
Copy link
Contributor Author

fty4 commented Jul 17, 2023

@cpanato I've updated the docs and forced-pushed the rebased commits.
(Required multiple force-push because of DCO)

@cpanato
Copy link
Member

cpanato commented Jul 17, 2023

this will be a breaking change for existing users, if they dont set the --target-branch in the config

@fty4
Copy link
Contributor Author

fty4 commented Jul 17, 2023

That is true - I already mentioned this in the PR body.

New users will still have to specify the parameter if this will not be changed.

@cpanato
Copy link
Member

cpanato commented Aug 16, 2023

when we release I will make a note in the release notes for this

Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

small request

examples/kind/README.md Outdated Show resolved Hide resolved
Co-authored-by: Carlos Tadeu Panato Junior <[email protected]>
Signed-off-by: Marco Lecheler <[email protected]>
@fty4 fty4 requested a review from cpanato September 15, 2023 10:18
@odysseu
Copy link

odysseu commented Oct 4, 2023

LGTM 👍

Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Nov 19, 2023
@cpanato cpanato changed the title chore: change default target-branch to main (Breaking change) chore: change default target-branch to main Nov 19, 2023
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

lgtm

@davidkarlsen
Copy link
Member

Lgtm. We can maybe bump version number a bit to signal breaking change.

@cpanato
Copy link
Member

cpanato commented Nov 19, 2023

Lgtm. We can maybe bump version number a bit to signal breaking change.

yes

@cpanato
Copy link
Member

cpanato commented Nov 19, 2023

let marinate this in the main branch for a while

@cpanato cpanato merged commit 61b5438 into helm:main Nov 19, 2023
4 checks passed
@fty4 fty4 deleted the chore/target-branch-main branch November 20, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] target-branch default should fallback to main from master (or visa versa)
7 participants