search

helm / chart-releaser

Hosting Helm Charts via GitHub Pages and Releases
Apache License 2.0
653 stars 106 forks source link

Peform single push on upload in case of multiple releases #314

Open amalgamm opened 1 year ago

amalgamm commented 1 year ago

Resolves the problem mentioned in this PR

Lets say you want to upload more than one release and place it near the index file. After the first push your second push will be rejected with an error "Updates were rejected because a pushed branch tip is behind its remote". So either you need to pull changes before push another release or push all releases once in the end of upload action.

In case of pulling changes and pushing every release separately you'll face github will launch as many "pages build and deployment" actions as there are new releases you want to upload. All except one will be canceled, but the list of actions will look like a mess.

So the most efficient way is to make single push in the end of upload.

amalgamm commented 12 months ago

@davidkarlsen can you have a look at this pr?

gaelgatelement commented 10 months ago

I just realized that my PR is a duplicate of this one, and this one is better as it fixes the tests. https://github.com/helm/chart-releaser/pull/336

Can we have someone take a look at this PR please ?

cpanato commented 9 months ago

does anyone can provide some manual tests on this? @amalgamm @gaelgatelement and show the logs and maybe the ci job

gaelgatelement commented 9 months ago

I'm using this fork in our own CI runs : https://github.com/vector-im/ess-starter-edition-core/actions/runs/6624187779

cpanato commented 9 months ago

thanks @gaelgatelement @amalgamm can you rebase and fix the lint issues?

cpanato commented 9 months ago

tested on my side and seems to work fine :)

ibice commented 8 months ago

Hi! I just run into the issue this PR fixes, it'd be cool to have it merged!

amalgamm commented 6 months ago

@cpanato I think I fixed the lint. Sorry it took so long

cpanato commented 5 months ago

Did you test the case with PackagesWithIndex set as well? not sure if will change any behavior

amalgamm commented 5 months ago

Did you test the case with PackagesWithIndex set as well? not sure if will change any behavior

I didn't but I think this case is covered by tests

jbrandli commented 4 months ago

please merge

marquesj2-ppb commented 2 months ago

Any way to get this merged? @cpanato

0xThresh commented 2 months ago

Sorry to tag again @cpanato but I'm running into the described issue as well and am hoping to see this merged. Thank you!

cpanato commented 1 month ago

I did a test using the latest chart-release and two chart updates and i see one push: https://github.com/cpanato/testing-ci-providers/actions/runs/9764682879/job/26953586398

Is this PR trying to update as well? If so, are we good, or am I missing something else?

cc @davidkarlsen