Skip to content

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

Merged
cpanato merged 4 commits intohelm:mainfrom
fty4:chore/target-branch-main
Nov 19, 2023
Merged

(Breaking change) chore: change default target-branch to main#510
cpanato merged 4 commits intohelm:mainfrom
fty4:chore/target-branch-main

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
@fty4 fty4 mentioned this pull request Jan 6, 2023
Merged
thisisrachelramos
thisisrachelramos approved these changes Feb 9, 2023
View reviewed changes
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
@fty4
chore: change default target-branch to main
d9cb34c 
closes #376

Signed-off-by: Marco Lecheler <[email protected]>
Signed-off-by: Marco Lecheler <[email protected]>
@fty4
docs: change default target-branch to main
294f73c 
Signed-off-by: Marco Lecheler <[email protected]>
Signed-off-by: Marco Lecheler <[email protected]>
@fty4
docs: update missing inclusive naming
5b1067d 
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

cpanato
cpanato requested changes Aug 18, 2023
View reviewed changes
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

@fty4 @cpanato
chore: update kind readme to use inclusive naming
d3e415c 
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 👍

This was referenced Oct 4, 2023
Merged
Merged
@jouve
Copy link
Contributor

jouve commented Oct 4, 2023

this can be tested here with

@jouve jouve mentioned this pull request Oct 4, 2023
Closed
@github-actions
Copy link

github-actions bot commented Nov 19, 2023

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
cpanato
cpanato approved these changes Nov 19, 2023
View reviewed changes
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

davidkarlsen commented Nov 19, 2023

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
@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

Reviewers

@davidkarlsen davidkarlsen davidkarlsen approved these changes

@cpanato cpanato cpanato approved these changes

+3 more reviewers

@joejulian joejulian joejulian approved these changes

@thisisrachelramos thisisrachelramos thisisrachelramos approved these changes

@odysseu odysseu odysseu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/S Stale

Projects

None yet

Milestone

No milestone

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

@fty4 @cpanato @odysseu @jouve @davidkarlsen @joejulian @thisisrachelramos